Match LoadArraySchemaResponse property names with core capnp spec. #473
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
NullHypothesis
approved these changes
Aug 6, 2024
| @@ -4816,9 +4816,9 @@ definitions: | |||
| description: Contains the latest schema and all schemas for the opened array | |||
| type: object | |||
| properties: | |||
| latest_array_schema: | |||
There was a problem hiding this comment.
Shall we add a comment here, explaining that this must match what core expects?
There was a problem hiding this comment.
Right, I was thinking along the same lines, how to avoid that very nasty issue in the future (great job finding it!!).
Since there is no automated way, maybe adding a comment in this file in a very visible way and updating our own documentation could help a bit: https://github.com/TileDB-Inc/TileDB-Cloud-REST/blob/7c23501a9d7d491bc09028b5cb8c223d68c306c7/docs/UpgradeCapnpSchemas.md
ypatia
approved these changes
Aug 6, 2024
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 the property names for LoadArraySchemaResponse to match those in Core. This came up after debugging issues unmarshaling LoadArraySchemaResponse in REST unit tests. IIUC the property names for the OpenAPI specification must match the field names in core's capnproto specification.
Alternatively I can open a PR in core to use snake case across the board, or open to other suggestions if there is another way to make this work without identical property names.