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.

Sign up for GitHub

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

Jump to bottom

Add new Java SDK #6656

Merged
merged 7 commits into from
Oct 4, 2023
Merged

Add new Java SDK #6656

merged 7 commits into from
Oct 4, 2023

Conversation

arielshaqed
Copy link
Contributor

@arielshaqed arielshaqed commented Sep 27, 2023 •
edited
Loading

  • In this PR:
    • Also publish new client
    • Test both presigned and "regular" modes
  • Publish this client
  • In future PRs:
    • Use published client in lakeFSFS build, or locally-installed build
    • Add remaining tests

@arielshaqed arielshaqed marked this pull request as draft September 27, 2023 11:21
@arielshaqed arielshaqed added java Pull requests that update Java code include-changelog PR description should be included in next release changelog v1.0.0-blocker Issues that should be closed before going out with v1.0.0 area/client/hadoopfs labels Sep 27, 2023
This was linked to issues Sep 27, 2023
[1.0] Forwards-compatible Java clients #6424
Closed
Add b/c test for Java API client #6581
Closed
@arielshaqed
Copy link
Contributor Author

Also fixes #6581 by adding a minimal test that the new client SDK is cool when it sees an unknown property. This does not really test lakeFS, it tests that OpenAPI-codegen does what we wanted it to do.

@arielshaqed arielshaqed force-pushed the feature/6424-single-arg-java-clients branch from 03184b5 to 0d9aef3 Compare September 27, 2023 12:29
@arielshaqed arielshaqed force-pushed the chore/test-lakefsfs-with-mock-server branch 2 times, most recently from 83d2190 to 1f34231 Compare September 27, 2023 15:50
@arielshaqed arielshaqed force-pushed the feature/6424-single-arg-java-clients branch from e13b87f to f125a1d Compare September 27, 2023 15:52
@arielshaqed arielshaqed force-pushed the chore/test-lakefsfs-with-mock-server branch from 1f34231 to 6ed5dc7 Compare September 27, 2023 15:53
@arielshaqed arielshaqed force-pushed the feature/6424-single-arg-java-clients branch 3 times, most recently from bc75a9d to 8e273bb Compare October 2, 2023 09:59
@arielshaqed arielshaqed marked this pull request as ready for review October 2, 2023 10:02
@Jonathan-Rosenberg
Copy link
Contributor

@arielshaqed can you please specify the files that you actually changed?
or the ones that you didn't 😉

@arielshaqed arielshaqed force-pushed the feature/6424-single-arg-java-clients branch 4 times, most recently from be4d891 to 7e0efc0 Compare October 2, 2023 12:58
@arielshaqed arielshaqed changed the title Add new Java SDK and use it in lakeFSFS Add new Java SDK Oct 2, 2023
@arielshaqed arielshaqed force-pushed the feature/6424-single-arg-java-clients branch from 96f0fbd to 47aabdf Compare October 2, 2023 14:11
arielshaqed
arielshaqed commented Oct 2, 2023
View reviewed changes
Copy link
Contributor Author

@arielshaqed arielshaqed left a comment

Choose a reason for hiding this comment

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

@Jonathan-Rosenberg PTAL :-)

I now:

  • Moved lakeFSFS changes to a separate PR
  • Told Linguist to fold java-legacy by default (see .gitattributes change!)
  • Rearranged some changes to be more logical by commit.

It should be easier to review.

@arielshaqed arielshaqed mentioned this pull request Oct 2, 2023
Merged
Jonathan-Rosenberg
Jonathan-Rosenberg reviewed Oct 3, 2023
View reviewed changes
api/java-gen-ignore
@@ -0,0 +1,3 @@
# Files to ignore (skip) when generating Java client
maven.yml
.github/
Copy link
Contributor

@Jonathan-Rosenberg Jonathan-Rosenberg Oct 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
.github/
.github/
git_push.sh

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now really re-done.

Jonathan-Rosenberg
Jonathan-Rosenberg requested changes Oct 3, 2023
View reviewed changes
Copy link
Contributor

@Jonathan-Rosenberg Jonathan-Rosenberg left a comment

Choose a reason for hiding this comment

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

LGTM except for a possibly missing open-api-ignore key.
Ping me if it's intentional

@arielshaqed arielshaqed mentioned this pull request Oct 3, 2023
Merged
An error occurred while trying to automatically change base from chore/test-lakefsfs-with-mock-server to master October 3, 2023 13:11
@arielshaqed arielshaqed mentioned this pull request Oct 3, 2023
Closed
@arielshaqed arielshaqed changed the base branch from chore/test-lakefsfs-with-mock-server to master October 3, 2023 13:29
@arielshaqed arielshaqed force-pushed the feature/6424-single-arg-java-clients branch from 78c52b4 to 2b7ab98 Compare October 4, 2023 07:47
@arielshaqed
Use OpenAPI codegen 7.0.0 to generate new Java client
6122b78 
* New Java client to fix #6424 will go in clients/java/.
* "Legacy" Java client moved to clients/java-legacy/.

The new client uses a single request-object parameter.

This commit does *not* include the new generated client, to allow Git to
display the rename of clients/java/ to clients/java-legacy/ on this commit.
@arielshaqed
Generated Java client
6c4d75d 
Backwards compatibility enabled by *not* verifying unknown properties.
@arielshaqed
Add legacy publish action
696c318 
The existing java-api-client "deploy" job should publish the new SDK.  Add a
job to publish the old API.  We will get rid of it after releasing 1.0.
@arielshaqed
Instruct Linguist to fold clients/java-legacy in GitHub
20e4b6f 
Now changes to this subdirectory will appear folded by default in PRs.
@arielshaqed
make gen to generate new clients
df4cba6
@arielshaqed arielshaqed force-pushed the feature/6424-single-arg-java-clients branch from 2b7ab98 to df4cba6 Compare October 4, 2023 08:03
@arielshaqed
Copy link
Contributor Author

I think that I added the desired openapi ignore key(?). PTAL...

@arielshaqed arielshaqed force-pushed the feature/6424-single-arg-java-clients branch from 89f4cc3 to 2230176 Compare October 4, 2023 08:55
@arielshaqed
Copy link
Contributor Author

PTAAAAAAAAAAAAL.

81dqs4

Jonathan-Rosenberg
Jonathan-Rosenberg approved these changes Oct 4, 2023
View reviewed changes
Copy link
Contributor

@Jonathan-Rosenberg Jonathan-Rosenberg left a comment

Choose a reason for hiding this comment

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

LGTM

@arielshaqed
Copy link
Contributor Author

Thanks!

@arielshaqed arielshaqed enabled auto-merge (squash) October 4, 2023 09:06
@arielshaqed arielshaqed merged commit f6c9737 into master Oct 4, 2023
32 checks passed
@arielshaqed arielshaqed deleted the feature/6424-single-arg-java-clients branch October 4, 2023 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jonathan-Rosenberg Jonathan-Rosenberg Jonathan-Rosenberg approved these changes

Assignees
No one assigned
Labels
area/client/hadoopfs include-changelog PR description should be included in next release changelog java Pull requests that update Java code v1.0.0-blocker Issues that should be closed before going out with v1.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add b/c test for Java API client [1.0] Forwards-compatible Java clients
2 participants
@arielshaqed @Jonathan-Rosenberg