Skip to content

IE-655: migrate skyflow-java beta to skyflow-flowvault-java GA 1.1.0 - #9

Open
skyflow-srivyshnavi wants to merge 5 commits into
mainfrom
IE-655
Open

skyflow-srivyshnavi wants to merge 5 commits into
mainfrom
IE-655

Conversation

@skyflow-srivyshnavi

@skyflow-srivyshnavi skyflow-srivyshnavi commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

IE-655: migrate skyflow-java beta to skyflow-flowvault-java GA 1.1.0
image

@piyushjain-skyflow piyushjain-skyflow left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good to me!

// Helper method to retry failed records with exponential backoff and jitter
private void retryFailedRecords(InsertRequest request,
Map<Object, Success> successMap,
private void retryFailedRecords(BulkInsertRequest request,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we use BulkInsertResponse.getRecordsToRetry() here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

public static final HashSet<Integer> RETRYABLE_ERROR_CODES = new HashSet<>(Arrays.asList(

Does it return records that cover these error codes @Devesh-Skyflow ?
I believe 429 is not covered.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes 429 is not covered

@koshy-skyflow

koshy-skyflow commented Aug 25, 2026

Copy link
Copy Markdown

Minor comments and findings, can be addressed later as well:

  1. The README prerequisites section still mentions Skyflow Java SDK 3.0.0-beta.6: https://github.com/SkyflowFoundry/spark-hive-utils/blob/main/spark/README.md#prerequisites

  2. UpsertOptions now supports updateType (UPDATE/REPLACE) in 1.1.0, but the wrapper never sets it, just flagging that there's currently no way for a caller use REPLACE as updateType on upsert.

  3. We should probably think of adding 409 error to RETRYABLE_ERROR_CODES as concurrent upsert conflicts for a non existing unique value surface as 409 http status weith error message "The request conflicted with another concurrent operation on the same resource. Retry the request. Contact Skyflow support with request ID %s if the issue persists."

  4. Are we planning to cover the Hive migration separately? Asking since the Hive utilities are in the same repo.

@skyflow-srivyshnavi

Copy link
Copy Markdown
Contributor Author

Minor comments and findings, can be addressed later as well:

  1. The README prerequisites section still mentions Skyflow Java SDK 3.0.0-beta.6: https://github.com/SkyflowFoundry/spark-hive-utils/blob/main/spark/README.md#prerequisites
  2. UpsertOptions now supports updateType (UPDATE/REPLACE) in 1.1.0, but the wrapper never sets it, just flagging that there's currently no way for a caller use REPLACE as updateType on upsert.
  3. We should probably think of adding 409 error to RETRYABLE_ERROR_CODES as concurrent upsert conflicts for a non existing unique value surface as 409 http status weith error message "The request conflicted with another concurrent operation on the same resource. Retry the request. Contact Skyflow support with request ID %s if the issue persists."
  4. Are we planning to cover the Hive migration separately? Asking since the Hive utilities are in the same repo.

Thanks Koshy

  1. Updated.
  2. Yes, not needed for Walmart. These are single column tables.
  3. Yes, added.
  4. They are not using Hive yet.

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.

4 participants