Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch RequestOption to accept long timeout only #6289

Merged
merged 1 commit into from
Sep 23, 2024

Conversation

rlazo
Copy link
Collaborator

@rlazo rlazo commented Sep 21, 2024

The RequestOptions class only customizable paramter, timeout, is now called timeoutInMillis and it's type is Long.

Using kotlin.time.Duration in our public API, while convenient, introduces an issue with Java code. In short, Duration is exposed as long in the JVM, and that long can represent time in milliseconds, or nanoseconds.

For clarity, we've fallback to long.

b/368715556

The `RequestOptions` class only customizable paramter, `timeout`, is
now called `timeoutInMillis` and it's type is `Long`.

Using `kotlin.time.Duration` in our public API, while convenient,
introduces an issue with Java code. In short, `Duration` is exposed as
`long` in the JVM, and that long can represent time in milliseconds, or
nanoseconds.

For clarity, we've fallback to long.

This change also removes the internal representation of
`RequestOptions`, which was a legacy implementation detail.
Copy link
Contributor

github-actions bot commented Sep 21, 2024

Javadoc Changes:
--- /home/runner/diff/original/firebase-kotlindoc/android/com/google/firebase/vertexai/type/RequestOptions.html	2024-09-21 00:40:34.129223666 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/android/com/google/firebase/vertexai/type/RequestOptions.html	2024-09-21 00:38:27.221189863 +0000
@@ -21,20 +21,25 @@
         </colgroup>
         <thead>
           <tr>
-            <th colspan="100%"><h3>Public fields</h3></th>
+            <th colspan="100%"><h3>Public constructors</h3></th>
           </tr>
         </thead>
         <tbody class="list">
           <tr>
-            <td><code>final @<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.time/-duration/index.html">Duration</a></code></td>
             <td>
-              <div><code><a href="/docs/reference/android/com/google/firebase/vertexai/type/RequestOptions.html#timeout()">timeout</a></code></div>
-              <p>the maximum amount of time for a request to take, from the first request to first response.</p>
+              <div><code><a href="/docs/reference/android/com/google/firebase/vertexai/type/RequestOptions.html#RequestOptions(kotlin.Long)">RequestOptions</a>(long&nbsp;timeoutInMillis)</code></div>
+              <p>Constructor for RequestOptions.</p>
             </td>
           </tr>
         </tbody>
       </table>
     </div>
+    <div class="list">
+      <h2>Public constructors</h2>
+      <div class="api-item"><a name="RequestOptions-kotlin.Long-"></a><a name="requestoptions"></a>
+        <h3 class="api-name" id="RequestOptions(kotlin.Long)">RequestOptions</h3>
+        <pre class="api-signature no-pretty-print">public&nbsp;<a href="/docs/reference/android/com/google/firebase/vertexai/type/RequestOptions.html#RequestOptions(kotlin.Long)">RequestOptions</a>(long&nbsp;timeoutInMillis)</pre>
+        <p>Constructor for RequestOptions.</p>
     <div class="devsite-table-wrapper">
       <table class="responsive">
         <colgroup>
@@ -43,40 +48,19 @@
         </colgroup>
         <thead>
           <tr>
-            <th colspan="100%"><h3>Public constructors</h3></th>
+                <th colspan="100%">Parameters</th>
           </tr>
         </thead>
         <tbody class="list">
           <tr>
+                <td><code>long&nbsp;timeoutInMillis</code></td>
             <td>
-              <div><code><a href="/docs/reference/android/com/google/firebase/vertexai/type/RequestOptions.html#RequestOptions(kotlin.time.Duration)">RequestOptions</a>(@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.time/-duration/index.html">Duration</a>&nbsp;timeout)</code></div>
-            </td>
-          </tr>
-          <tr>
-            <td>
-              <div><code><a href="/docs/reference/android/com/google/firebase/vertexai/type/RequestOptions.html#RequestOptions(kotlin.Long)">RequestOptions</a>(<a href="https://developer.android.com/reference/kotlin/java/lang/Long.html">Long</a>&nbsp;timeout)</code></div>
+                  <p>the maximum amount of time, in milliseconds, for a request to take, from the first request to first response.</p>
             </td>
           </tr>
         </tbody>
       </table>
     </div>
-    <div class="list">
-      <h2>Public fields</h2>
-      <div class="api-item"><a name="getTimeout()"></a><a name="setTimeout()"></a><a name="getTimeout--"></a><a name="setTimeout--"></a>
-        <h3 class="api-name" id="timeout()">timeout</h3>
-        <pre class="api-signature no-pretty-print">public&nbsp;final&nbsp;@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.time/-duration/index.html">Duration</a>&nbsp;<a href="/docs/reference/android/com/google/firebase/vertexai/type/RequestOptions.html#timeout()">timeout</a></pre>
-        <p>the maximum amount of time for a request to take, from the first request to first response.</p>
-      </div>
-    </div>
-    <div class="list">
-      <h2>Public constructors</h2>
-      <div class="api-item"><a name="RequestOptions-kotlin.time.Duration-"></a><a name="requestoptions"></a>
-        <h3 class="api-name" id="RequestOptions(kotlin.time.Duration)">RequestOptions</h3>
-        <pre class="api-signature no-pretty-print">public&nbsp;<a href="/docs/reference/android/com/google/firebase/vertexai/type/RequestOptions.html#RequestOptions(kotlin.time.Duration)">RequestOptions</a>(@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.time/-duration/index.html">Duration</a>&nbsp;timeout)</pre>
-      </div>
-      <div class="api-item"><a name="RequestOptions-kotlin.Long-"></a><a name="requestoptions"></a>
-        <h3 class="api-name" id="RequestOptions(kotlin.Long)">RequestOptions</h3>
-        <pre class="api-signature no-pretty-print">public&nbsp;<a href="/docs/reference/android/com/google/firebase/vertexai/type/RequestOptions.html#RequestOptions(kotlin.Long)">RequestOptions</a>(<a href="https://developer.android.com/reference/kotlin/java/lang/Long.html">Long</a>&nbsp;timeout)</pre>
       </div>
     </div>
   </body>
--- /home/runner/diff/original/firebase-kotlindoc/kotlin/com/google/firebase/vertexai/type/RequestOptions.html	2024-09-21 00:40:34.122223663 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/kotlin/com/google/firebase/vertexai/type/RequestOptions.html	2024-09-21 00:38:27.211189862 +0000
@@ -27,17 +27,19 @@
         <tbody class="list">
           <tr>
             <td>
-              <div><code><a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/RequestOptions.html#RequestOptions(kotlin.time.Duration)">RequestOptions</a>(timeout:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.time/-duration/index.html">Duration</a>)</code></div>
-            </td>
-          </tr>
-          <tr>
-            <td>
-              <div><code><a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/RequestOptions.html#RequestOptions(kotlin.Long)">RequestOptions</a>(timeout:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-long/index.html">Long</a>?)</code></div>
+              <div><code><a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/RequestOptions.html#RequestOptions(kotlin.Long)">RequestOptions</a>(timeoutInMillis:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-long/index.html">Long</a>)</code></div>
+              <p>Constructor for RequestOptions.</p>
             </td>
           </tr>
         </tbody>
       </table>
     </div>
+    <div class="list">
+      <h2>Public constructors</h2>
+      <div class="api-item"><a name="RequestOptions-kotlin.Long-"></a><a name="requestoptions"></a>
+        <h3 class="api-name" id="RequestOptions(kotlin.Long)">RequestOptions</h3>
+        <pre class="api-signature no-pretty-print"><a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/RequestOptions.html#RequestOptions(kotlin.Long)">RequestOptions</a>(timeoutInMillis:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-long/index.html">Long</a> = 180.seconds.inWholeMilliseconds)</pre>
+        <p>Constructor for RequestOptions.</p>
     <div class="devsite-table-wrapper">
       <table class="responsive">
         <colgroup>
@@ -46,37 +48,19 @@
         </colgroup>
         <thead>
           <tr>
-            <th colspan="100%"><h3>Public properties</h3></th>
+                <th colspan="100%">Parameters</th>
           </tr>
         </thead>
         <tbody class="list">
           <tr>
-            <td><code><a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.time/-duration/index.html">Duration</a></code></td>
+                <td><code>timeoutInMillis:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-long/index.html">Long</a> = 180.seconds.inWholeMilliseconds</code></td>
             <td>
-              <div><code><a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/RequestOptions.html#timeout()">timeout</a></code></div>
-              <p>the maximum amount of time for a request to take, from the first request to first response.</p>
+                  <p>the maximum amount of time, in milliseconds, for a request to take, from the first request to first response.</p>
             </td>
           </tr>
         </tbody>
       </table>
     </div>
-    <div class="list">
-      <h2>Public constructors</h2>
-      <div class="api-item"><a name="RequestOptions-kotlin.time.Duration-"></a><a name="requestoptions"></a>
-        <h3 class="api-name" id="RequestOptions(kotlin.time.Duration)">RequestOptions</h3>
-        <pre class="api-signature no-pretty-print"><a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/RequestOptions.html#RequestOptions(kotlin.time.Duration)">RequestOptions</a>(timeout:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.time/-duration/index.html">Duration</a>)</pre>
-      </div>
-      <div class="api-item"><a name="RequestOptions-kotlin.Long-"></a><a name="requestoptions"></a>
-        <h3 class="api-name" id="RequestOptions(kotlin.Long)">RequestOptions</h3>
-        <pre class="api-signature no-pretty-print"><a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/RequestOptions.html#RequestOptions(kotlin.Long)">RequestOptions</a>(timeout:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-long/index.html">Long</a>? = Long.MAX_VALUE)</pre>
-      </div>
-    </div>
-    <div class="list">
-      <h2>Public properties</h2>
-      <div class="api-item"><a name="getTimeout()"></a><a name="setTimeout()"></a><a name="getTimeout--"></a><a name="setTimeout--"></a>
-        <h3 class="api-name" id="timeout()">timeout</h3>
-        <pre class="api-signature no-pretty-print">val&nbsp;<a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/RequestOptions.html#timeout()">timeout</a>:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.time/-duration/index.html">Duration</a></pre>
-        <p>the maximum amount of time for a request to take, from the first request to first response.</p>
       </div>
     </div>
   </body>

@google-oss-bot
Copy link
Contributor

1 Warning
⚠️ Did you forget to add a changelog entry? (Add the 'no-changelog' label to the PR to silence this warning.)

Generated by 🚫 Danger

@google-oss-bot
Copy link
Contributor

@google-oss-bot
Copy link
Contributor

Size Report 1

Affected Products

  • firebase-vertexai

    TypeBase (da0eefa)Merge (353c7c7)Diff
    aar489 kB487 kB-1.44 kB (-0.3%)
    apk (release)9.30 MB9.30 MB-1.31 kB (-0.0%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/wFq89bwQcL.html

Copy link
Contributor

Unit Test Results

  16 files  +    8    16 suites  +8   16s ⏱️ -12s
108 tests +  86  108 ✔️ +  86  0 💤 ±0  0 ±0 
216 runs  +172  216 ✔️ +172  0 💤 ±0  0 ±0 

Results for commit 6a4e35c. ± Comparison against base commit da0eefa.

This pull request removes 22 and adds 108 tests. Note that renamed tests count towards both.
com.google.firebase.crashlytics.internal.common.CrashlyticsControllerRobolectricTest ‑ testDoCloseSession_disabledAnrs_doesNotPersistsAppExitInfo
com.google.firebase.crashlytics.internal.common.CrashlyticsControllerRobolectricTest ‑ testDoCloseSession_enabledAnrs_doesNotPersistsAppExitInfoIfItDoesntExist
com.google.firebase.crashlytics.internal.common.CrashlyticsControllerRobolectricTest ‑ testDoCloseSession_enabledAnrs_persistsAppExitInfoIfItExists
com.google.firebase.crashlytics.internal.common.DataCollectionArbiterRobolectricTest ‑ testDefaultDataCollection_usedWhenNoOverrideOrManifestSetting
com.google.firebase.crashlytics.internal.common.DataCollectionArbiterRobolectricTest ‑ testManifestMetadata_respectedWhenNoOverride
com.google.firebase.crashlytics.internal.common.DataCollectionArbiterRobolectricTest ‑ testSetCrashlyticsDataCollectionEnabled_overridesOtherSettings
com.google.firebase.crashlytics.internal.common.SessionReportingCoordinatorRobolectricTest ‑ testAppExitInfoEvent_notPersistIfAnrBeforeSession
com.google.firebase.crashlytics.internal.common.SessionReportingCoordinatorRobolectricTest ‑ testAppExitInfoEvent_notPersistIfAppExitInfoNotAnrButWithinSession
com.google.firebase.crashlytics.internal.common.SessionReportingCoordinatorRobolectricTest ‑ testAppExitInfoEvent_persistIfAnrWithinSession
com.google.firebase.crashlytics.internal.common.SessionReportingCoordinatorRobolectricTest ‑ testAppExitInfoEvent_persistIfAnrWithinSession_multipleAppExitInfo
…
com.google.firebase.vertexai.SchemaTests ‑ basic schema declaration
com.google.firebase.vertexai.SchemaTests ‑ full schema declaration
com.google.firebase.vertexai.StreamingSnapshotTests ‑ citation parsed correctly
com.google.firebase.vertexai.StreamingSnapshotTests ‑ empty content
com.google.firebase.vertexai.StreamingSnapshotTests ‑ http errors
com.google.firebase.vertexai.StreamingSnapshotTests ‑ image rejected
com.google.firebase.vertexai.StreamingSnapshotTests ‑ invalid api key
com.google.firebase.vertexai.StreamingSnapshotTests ‑ invalid json
com.google.firebase.vertexai.StreamingSnapshotTests ‑ long reply
com.google.firebase.vertexai.StreamingSnapshotTests ‑ malformed content
…

@rlazo rlazo enabled auto-merge (squash) September 21, 2024 03:54
@rlazo rlazo added this to the vertexai-ga milestone Sep 21, 2024
@rlazo rlazo merged commit f0dd60a into main Sep 23, 2024
31 of 32 checks passed
@rlazo rlazo deleted the rl.request.option.duration branch September 23, 2024 15:23
@firebase firebase locked and limited conversation to collaborators Oct 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants