Skip to content

Conversation

@cdkrot
Copy link
Contributor

@cdkrot cdkrot commented Jan 15, 2024

What changes were proposed in this pull request?

Make addArtifact API retrying on errors.

Note this is safe operation since addArtifact is idempotent operation (#43314)

Why are the changes needed?

For the same reasons as we make other API retryable.

Does this PR introduce any user-facing change?

Yes

How was this patch tested?

Added test.

Testing by hand against custom spark server.

Was this patch authored or co-authored using generative AI tooling?

No & never

@cdkrot
Copy link
Contributor Author

cdkrot commented Jan 15, 2024

cc @vicennial @nija-at @hvanhovell

Copy link
Contributor

@vicennial vicennial left a comment

Choose a reason for hiding this comment

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

LGTM!

.connectionString(s"sc://localhost:${server.getPort}")
.interceptor(new ClientInterceptor {
override def interceptCall[ReqT, RespT](
methodDescriptor: MethodDescriptor[ReqT, RespT],
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT style

Copy link
Contributor

@hvanhovell hvanhovell left a comment

Choose a reason for hiding this comment

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

LGTM

@cdkrot
Copy link
Contributor Author

cdkrot commented Jan 15, 2024

Tests green!

@HyukjinKwon
Copy link
Member

Merged to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants