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.

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

(WIP) Fix #1469: (createTable deser exposes table command) add tests, change deserialization logic #1470

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tatu-at-datastax
Copy link
Contributor

@tatu-at-datastax tatu-at-datastax commented Sep 27, 2024

What this PR does:

Adds reproduction of #1469 to show the issues, and changes deserialization to defer validation failures after APIFeature check.

Which issue(s) this PR fixes:
Fixes #1469

Checklist

  • Changes manually tested
  • Automated Tests added/updated
  • Documentation added/updated
  • CLA Signed: DataStax CLA

@tatu-at-datastax tatu-at-datastax self-assigned this Sep 27, 2024
@tatu-at-datastax tatu-at-datastax changed the title (WIP) Add failing test to show #1469 Fix #1469: add tests, change deserialization logic Sep 27, 2024
@tatu-at-datastax tatu-at-datastax marked this pull request as ready for review September 27, 2024 19:35
@tatu-at-datastax tatu-at-datastax requested a review from a team as a code owner September 27, 2024 19:35
@tatu-at-datastax
Copy link
Contributor Author

Fix here works but not completely convinced we want to take this approach -- however, if we do want to solve the issue something like this is needed.

@tatu-at-datastax tatu-at-datastax changed the title Fix #1469: add tests, change deserialization logic Fix #1469: (createTable deser exposes table command) add tests, change deserialization logic Oct 1, 2024
@tatu-at-datastax
Copy link
Contributor Author

PR hard to merge due to concurrent refactorings -- leaving open in case it needs to be revived; if so, will probably need to re-create.

@tatu-at-datastax tatu-at-datastax changed the title Fix #1469: (createTable deser exposes table command) add tests, change deserialization logic (WIP) Fix #1469: (createTable deser exposes table command) add tests, change deserialization logic Oct 8, 2024
@tatu-at-datastax tatu-at-datastax marked this pull request as draft October 8, 2024 18:58
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.

Do not expose table related exception if feature is not enabled.
1 participant