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

feat(clients): endpoint level timeout part 2 #4318

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

Fluf22
Copy link
Contributor

@Fluf22 Fluf22 commented Jan 8, 2025

🧭 What and Why

🎟 JIRA Ticket: DI-3322 & DI-3327 & DI-3328 & DI-3329 & DI-3330 & DI-3331 & DI-3332 & DI-3333 & DI-3382

Changes included:

  • Swift
  • Kotlin
  • Java
  • Scala
  • C#
  • PHP
  • Go
  • Dart?

🧪 Test

@Fluf22 Fluf22 self-assigned this Jan 8, 2025
@algolia-bot
Copy link
Collaborator

algolia-bot commented Jan 8, 2025

🪓 The generated code will be pushed at the end of the CI.

Action triggered by commit 7990e43306ef244c7043a5411a181a45c04d93ec.

Please do not push any generated code to this pull request.

@Fluf22 Fluf22 force-pushed the fix/endpoint-level-timeouts branch 7 times, most recently from fae535d to 52dfa8f Compare January 9, 2025 16:18
@Fluf22 Fluf22 force-pushed the fix/endpoint-level-timeouts branch from 52dfa8f to 3fb27f1 Compare January 9, 2025 16:19
@Fluf22 Fluf22 force-pushed the fix/endpoint-level-timeouts branch from 3fb27f1 to 1da1e81 Compare January 9, 2025 17:08
@Fluf22 Fluf22 force-pushed the fix/endpoint-level-timeouts branch from 9e7e6a9 to e53bc13 Compare January 10, 2025 10:00
@Fluf22
Copy link
Contributor Author

Fluf22 commented Jan 10, 2025

After the next release, I will rebase this PR on main so the CI for Java 17 can pass

@Fluf22 Fluf22 marked this pull request as ready for review January 10, 2025 10:13
@Fluf22 Fluf22 requested a review from a team as a code owner January 10, 2025 10:13
@millotp
Copy link
Collaborator

millotp commented Jan 10, 2025

After the next release, I will rebase this PR on main so the CI for Java 17 can pass

Or you can add [skip-bc] at the end of the title

@Fluf22
Copy link
Contributor Author

Fluf22 commented Jan 10, 2025

Or you can add [skip-bc] at the end of the title

Yeah but it's not an urgent PR? I don't think it will be merged before next week, so let's wait?

Copy link
Collaborator

@millotp millotp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is huge !

@@ -6,6 +6,7 @@ import {{invokerPackage}}.ApiClient;
import {{invokerPackage}}.config.ClientOptions;

import com.fasterxml.jackson.core.type.TypeReference;
import org.jetbrains.annotations.Nullable;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we already use javax.annotations.Nonnull in this file

Suggested change
import org.jetbrains.annotations.Nullable;
import javax.annotations.Nullable;

@@ -32,4 +42,7 @@ public RequestOptions()
QueryParameters = new Dictionary<string, object>();
Headers = new Dictionary<string, string>();
}

// overload + operator
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this comment ?


@Override
public void execute(Template.Fragment fragment, Writer writer) throws IOException {
writer.write(Integer.toString(Integer.parseInt(fragment.execute()) / 1000));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with this lambda, can you remove the logic in Timeouts.enrichBundle and move it to the mustache instead ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants