Skip to content

Conversation

@DimitrisStaratzis
Copy link
Contributor

@DimitrisStaratzis DimitrisStaratzis commented Jul 2, 2024

I used the openapi-v2.yaml to generate the code for the v2 routes. I then restructured the packages to include v1 and v2.

This is the beginning. In following PRs I will also include the v2 routes in our wrapping classes and examples.

@DimitrisStaratzis DimitrisStaratzis requested review from KiterLuc, antalakas, ihnorton and teo-tsirpanis and removed request for teo-tsirpanis July 2, 2024 16:24
Comment on lines +5 to +8
import io.tiledb.cloud.rest_api.v1.ApiException;
import io.tiledb.cloud.rest_api.v1.api.GroupsApi;
import io.tiledb.cloud.rest_api.v1.api.ArrayApi;
import io.tiledb.cloud.rest_api.v1.model.*;
Copy link
Member

Choose a reason for hiding this comment

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

This will be a breaking change. Not sure if you want that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will be a breaking change yes, necessary in my opinion to include the new code. Otherwise the packaging will be unclear

Copy link
Member

Choose a reason for hiding this comment

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

I see. I'm leaving it to others to make the decision on the breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the comment!

Copy link
Member

Choose a reason for hiding this comment

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

Per discussion, we're going to alias the v1 elements and provide a deprecation pathway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Class aliases are not available in Java but I did the following. I created the v1 directory and also left the v1 files in their previous directory. So I pretty match did a copy-paste and we ended up with multiple duplicates. In the files that are in the unwanted directory I added a deprecation warning. These files will be removed in our next release.

Copy link
Contributor Author

@DimitrisStaratzis DimitrisStaratzis Jul 8, 2024

Choose a reason for hiding this comment

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

The deprecation warning is:

**
  * @deprecated This class is now moved to io.tiledb.cloud.rest_api.v1.api
  */
 @Deprecated

Copy link
Member

@ihnorton ihnorton Jul 8, 2024

Choose a reason for hiding this comment

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

Thanks. Without an aliasing mechanism (java 🙃), this will be an all-or-nothing change: users will need to update all uses at once, because the v1.* types will be semantically distinct even if they are nominally identical. So, if at all possible, we should wait two libTileDB releases before removing the old signatures in order to follow our general deprecation policy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good!

@DimitrisStaratzis DimitrisStaratzis merged commit ab5665a into master Jul 8, 2024
@DimitrisStaratzis DimitrisStaratzis deleted the dstara/sc-50152/add-v2-routes branch July 8, 2024 20:02
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