Update to use post_array_schema_from_rest.
#5181
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds all of the Capnp messages, serialization code, rest client functions, and server side request processing for handling the new load array schema handler. Notably, this does not update `tiledb_array_schema_load` to use these new functions because that cannot happen until after TileDb-Cloud-REST is deployed with a libtiledb that contains this commit.
This change enables the REST POST endpoint for loading array schemas from the REST server as well as enables the load_schema_with_enumerations behavior both for rest and local storage.
Originally I just slapped a tiledb_array_schema_load_with_enumerations and a hidden boolean on the load. This approach will prevent us from having to deprecate APIs in the future if we ever need to change the list of options for loading array schema.
…-load-array-schema-request
…-load-array-request
+ Fix bug from using incorrect timestamps.
ypatia
reviewed
Jul 12, 2024
ypatia
reviewed
Jul 12, 2024
shaunrd0
commented
Jul 19, 2024
Yep marked ready for review, looking at CI failures now |
nickvigilante
requested changes
Jul 22, 2024
shaunrd0
force-pushed
the
pd/sc-32991/use-handle-load-array-request
branch
from
e008f81 to
7f7fa1d
Compare
shaunrd0
force-pushed
the
pd/sc-32991/use-handle-load-array-request
branch
from
7f7fa1d to
aa494ce
Compare
|
Moving to draft to not accidentally merge before fixing the |
nickvigilante
approved these changes
Jul 24, 2024
…-load-array-request
+ Fixes a test failure in REST where only the latest schema would have enumerations loaded in the response.
|
I think this PR in its current format creates a chicken and egg situation. Long story short we can't be introducing CAPNP changes in the same PR as we are enabling a REST request that will need to be handled server-side by deserializing that changed CAPNP model. Not at all confusing 😅 As a rule of thumb CAPNP changes need to happen in a previous release than the one that sends a request to the route that will be using that CAPNP.
Hope it makes sense and sorry I didn't catch that earlier :( |
KiterLuc
pushed a commit
that referenced
this pull request
Aug 27, 2024
…5237) This factors out serialization and API changes in #5181 that are required for the HandleGetArraySchema route. These changes will need to be available on REST before we can enable the new route for loading the array schema. There is a quick summary of the changes required in [SC-52877](https://app.shortcut.com/tiledb-inc/story/52877/core-serialization-changes-for-loadarrayschema-models). --- TYPE: IMPROVEMENT DESC: Add serialization and API changes for post_array_schema_from_rest.
…-load-array-request
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This updates internal
get_array_schema_from_restcalls topost_array_schema_from_restadded in #4399. This also fixes array schema evolution time traveling issues reported in SC-49827 caused by not using timestamps to open the array fortiledb://URIs.[sc-32991]
TYPE: NO_HISTORY
DESC: Update to use
post_array_schema_from_rest.