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

feat!: improve query to support dynein format #187

Merged
merged 1 commit into from
Apr 14, 2024

Conversation

StoneDot
Copy link
Contributor

@StoneDot StoneDot commented Oct 16, 2023

Issue #, if available:
Implement #163

Description of changes:
This PR improves the parsing for the sort key condition. The current implementation cannot handle the sort key containing spaces. This PR fixes this and provides a way that is compatible with the dynein format.

Improvements contain the following features:

  • Introduce two formats for --sort-key: strict format and non-strict.
  • You can now query any condition DynamoDB supports using the strict format.
  • The non-strict format allows the user less typing and types agnostic input.
  • The non-strict format retains the same experience as much as possible.
  • Add --strict and --non-strict options to change the usage of non-strict.
  • Add configuration option and query.strict_mode to change the default mode.

BREAKING CHANGE: Parsing --sort-key is incompatible with the previous one.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@StoneDot StoneDot requested a review from kzym-w October 19, 2023 09:29
@StoneDot StoneDot added the enhancement New feature or request label Mar 1, 2024
@StoneDot StoneDot linked an issue Mar 1, 2024 that may be closed by this pull request
@StoneDot StoneDot force-pushed the improve-query-parse branch from 5ac4fde to 92ce166 Compare March 1, 2024 19:58
@StoneDot StoneDot force-pushed the improve-query-parse branch 2 times, most recently from 9a81b4d to 456ef23 Compare March 25, 2024 03:34
@StoneDot StoneDot marked this pull request as ready for review March 25, 2024 03:34
@StoneDot StoneDot changed the title wip: improve query option improve query option Mar 25, 2024
@StoneDot StoneDot force-pushed the improve-query-parse branch from 456ef23 to 7eca4dd Compare March 28, 2024 10:00
@StoneDot StoneDot requested a review from a team as a code owner March 28, 2024 10:00
@tmyoda tmyoda self-requested a review March 30, 2024 10:50
Copy link
Contributor

@tmyoda tmyoda left a comment

Choose a reason for hiding this comment

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

Thank you for adding awesome new feature!
I've left some minor feedback. Could you please check it?

docs/query.md Outdated Show resolved Hide resolved
docs/query.md Outdated Show resolved Hide resolved
docs/query.md Outdated Show resolved Hide resolved
docs/query.md Outdated Show resolved Hide resolved
src/cmd.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/parser.rs Show resolved Hide resolved
src/parser.rs Outdated Show resolved Hide resolved
src/cmd.rs Outdated Show resolved Hide resolved
tests/query.rs Outdated Show resolved Hide resolved
@StoneDot StoneDot changed the title improve query option feat!: improve query to support dynein format Apr 2, 2024
@StoneDot StoneDot force-pushed the improve-query-parse branch from 5e6eb1e to df93fa4 Compare April 2, 2024 16:19
@StoneDot
Copy link
Contributor Author

StoneDot commented Apr 2, 2024

@tmyoda Thank you for reviewing thoroughly. Based on your feedback, I updated the commit and commented. Could you please check them?

@StoneDot StoneDot force-pushed the improve-query-parse branch from 465b3c6 to 78e56d8 Compare April 2, 2024 16:38
@StoneDot StoneDot requested a review from tmyoda April 9, 2024 15:27
@StoneDot StoneDot force-pushed the improve-query-parse branch from 78e56d8 to d75ed8d Compare April 9, 2024 15:55
src/parser.rs Outdated Show resolved Hide resolved
tests/query.rs Outdated Show resolved Hide resolved
@tmyoda
Copy link
Contributor

tmyoda commented Apr 9, 2024

@StoneDot Thank you for updating the PR! I have left 2 comments. If you could check them, that would be great.

Copy link
Contributor

@tmyoda tmyoda left a comment

Choose a reason for hiding this comment

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

I found some typos in parser.rs. I apologize for submitting the reviews repeatedly.

src/parser.rs Outdated Show resolved Hide resolved
src/parser.rs Outdated Show resolved Hide resolved
src/parser.rs Outdated Show resolved Hide resolved
This improves the query command to support the dynein format in --sort-key.
Improvements contain the following features:

* Introduce two formats for --sort-key: strict format and non-strict.
* You can now query any condition DynamoDB supports using the strict format.
* The non-strict format allows the user less typing and types agnostic input.
* The non-strict format retains the same experience as much as possible.
* Add --strict and --non-strict options to change the usage of non-strict.
* Add configuration option and query.strict_mode to change the default mode.

BREAKING CHANGE: Parsing --sort-key is incompatible with the previous one.

Signed-off-by: Hiroaki Goto <[email protected]>
@StoneDot StoneDot force-pushed the improve-query-parse branch from d75ed8d to 9a250c3 Compare April 11, 2024 16:23
@tmyoda
Copy link
Contributor

tmyoda commented Apr 14, 2024

Thank you for repeatedly making the requested changes! It looks good to me. The changes are well-organized and clear despite the extensive work. I'm impressed by your attention to detail.

@tmyoda tmyoda merged commit c6f023b into awslabs:main Apr 14, 2024
3 checks passed
@StoneDot StoneDot mentioned this pull request Apr 15, 2024
23 tasks
@StoneDot StoneDot deleted the improve-query-parse branch April 19, 2024 08:15
KIrie-0217 pushed a commit to KIrie-0217/dynein that referenced this pull request May 15, 2024
This improves the query command to support the dynein format in --sort-key.
Improvements contain the following features:

* Introduce two formats for --sort-key: strict format and non-strict.
* You can now query any condition DynamoDB supports using the strict format.
* The non-strict format allows the user less typing and types agnostic input.
* The non-strict format retains the same experience as much as possible.
* Add --strict and --non-strict options to change the usage of non-strict.
* Add configuration option and query.strict_mode to change the default mode.

BREAKING CHANGE: Parsing --sort-key is incompatible with the previous one.

Signed-off-by: Hiroaki Goto <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement sophisticated parser for query command
2 participants