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

Disable missing key warnings when expected #1379

Merged
merged 1 commit into from
Feb 29, 2024
Merged

Conversation

IvoDD
Copy link
Collaborator

@IvoDD IvoDD commented Feb 28, 2024

  • Sets opts.dont_warn_about_missing_key=true when missing keys are expected (For example when writing a previously missing symbol it is expected to try to read a missing ref key)

Reference Issues/PRs

What does this implement or fix?

Any other comments?

Checklist

Checklist for code changes...
  • Have you updated the relevant docstrings, documentation and copyright notice?
  • Is this contribution tested against all ArcticDB's features?
  • Do all exceptions introduced raise appropriate error messages?
  • Are API changes highlighted in the PR description?
  • Is the PR labelled as enhancement or bug so it appears in autogenerated release notes?

- Also adds a minor clarifying rename in s3 details
@IvoDD IvoDD self-assigned this Feb 28, 2024
@muhammadhamzasajjad
Copy link
Collaborator

@IvoDD, whatever fix you decide to use, please ensure that it is consistent between all the storages. Currently, you are removing the warning only from s3, but the same warning is also logged in other storages. If you decide to stick with this solution, please remove the warning from other storages too.

Instead if you decide to set opts.dont_warn_about_missing_key to true as a solution, then the behaviour should already be consistent between the storages.

@poodlewars
Copy link
Collaborator

As discussed in Slack let's set opts.dont_warn_about_missing_key=true on read_symbol_ref and leave the storage backends untouched. I'm assuming that read_symbol_ref is the only additional place we wish to suppress the warning, but worth checking if we should add it elsewhere too.

@IvoDD IvoDD force-pushed the s3-remove-unneeded-warnings branch from aed0210 to 3597716 Compare February 28, 2024 15:57
@IvoDD IvoDD changed the title Removes unneded warnings when using s3 Disable missing key warnings when expected Feb 28, 2024
@IvoDD IvoDD merged commit 1d4f4f9 into master Feb 29, 2024
110 checks passed
@IvoDD IvoDD deleted the s3-remove-unneeded-warnings branch February 29, 2024 08:44
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.

3 participants