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.

Sign up for GitHub

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

Jump to bottom

clp-s: Return success code from cli for legitimate queries proven to match no results before decompression and scan. #268

Merged
merged 1 commit into from
Feb 9, 2024
Merged

clp-s: Return success code from cli for legitimate queries proven to match no results before decompression and scan. #268

merged 1 commit into from
Feb 9, 2024

Conversation

kirkrodrigues
Copy link
Member

@kirkrodrigues kirkrodrigues commented Feb 9, 2024 •
edited
Loading

Description

clp-s currently treats no timestamp/schema matches as an error which causes searches from the UI (#250) to fail when no records match the user's query. This PR changes them to success returns and performs some other related clean-up of the return values / log messages:

  • Make error return values consistent.
  • Clean-up log messages on lex/parse error.

Validation performed

  • Validated compression, decompression, and search of a test file.
  • Validated that a search that matched no schemas produced a successful return.
  • Validated that a search that matched no timestamp ranges produced a successful return.
  • Validated that an invalid query only printed a log about the lex/parse error and produced an error return.
  • Validated that missing arguments produced an error return.

@kirkrodrigues
Clean-up clp-s' CLI error behaviour:
8efe0b2 
- Make no timestamp/schema matches a success return.
- Make error return values consistent.
- Clean-up log messages on lex/parse error.
gibber9809
gibber9809 approved these changes Feb 9, 2024
View reviewed changes
Copy link
Contributor

@gibber9809 gibber9809 left a comment

Choose a reason for hiding this comment

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

LGTM. Maybe clp-s: Return success code from cli for legitimate queries proven to match no results before decompression and scan. (#268) for a commit message?

@kirkrodrigues kirkrodrigues merged commit c0969d5 into y-scope:main Feb 9, 2024
5 checks passed
@kirkrodrigues kirkrodrigues deleted the clp-s-retval branch February 9, 2024 02:56
@kirkrodrigues kirkrodrigues changed the title Clean-up clp-s' CLI error behaviour: clp-s: Return success code from cli for legitimate queries proven to match no results before decompression and scan. Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gibber9809 gibber9809 gibber9809 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kirkrodrigues @gibber9809