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

new-backend: adds useless warnings #4675

Closed
markus2330 opened this issue Nov 12, 2022 · 7 comments
Closed

new-backend: adds useless warnings #4675

markus2330 opened this issue Nov 12, 2022 · 7 comments
Labels

Comments

@markus2330
Copy link
Contributor

Steps to Reproduce the Problem

Make a checker plugin fail, e.g.:

kdb mount type.toml user:/tests/type toml type
kdb meta-set user:/tests/type/latitude type double

Expected Result

Sorry, module type issued the error C03200:
Validation Semantic: The type 'double' failed to match for 'user:/tests/type/latitude' with string ''

Actual Result

 Sorry, 2 warnings were issued ;(
 1: Module kdb issued the warning C01320:
        Interface: Calling the kdbSet function for the backend plugin ('backend') of the mountpoint 'user:/tests/type' has failed during the PRE_STORAGE phase.
 2: Module kdb issued the warning C01320:
        Interface: The PRE_STORAGE phase of kdbSet() has failed. See warnings for details.
Sorry, module type issued the error C03200:
Validation Semantic: The type 'double' failed to match for 'user:/tests/type/latitude' with string ''
@kodebach
Copy link
Member

I don't see the warnings as completely useless, they tell you which backend failed and during which phase it failed. It also covers the case, where a plugin returns an error status, but doesn't set an error.

Maybe we put these warnings behind some kind of debug flag. However, I don't see that as an urgent issue.

@kodebach kodebach removed their assignment Nov 12, 2022
@kodebach
Copy link
Member

Note: I unassigned myself, because I don't see any reason why I specifically have to do this. Anybody should be able to fix this issue. By searching for text fragments from the warning it should be quite obvious where the warnings are produced.

@markus2330
Copy link
Contributor Author

Logging and warnings should not be confused. I agree that logs about what happens in which phase can be useful. Warnings are, however, not designed for logging.

This is simply something that wasn't found in the review of #4187.

@kodebach
Copy link
Member

I agree that logging and warnings are different, but at least for "the case, where a plugin returns an error status, but doesn't set an error", the warning/error makes sense.

So probably the solution is (when an error status is returned)

  1. Check if the plugin set an error
  2. If it did, use logging
  3. If it did not, add a generic error like the one we have now as a warning

@markus2330
Copy link
Contributor Author

I agree that logging and warnings are different, but at least for "the case, where a plugin returns an error status, but doesn't set an error", the warning/error makes sense.

I fully agree, I think we can use ELEKTRA_SET_INTERFACE_ERROR here.

Copy link

I mark this stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping by writing a message here or create a new issue with the remainder of this issue.
Thank you for your contributions 💖

@github-actions github-actions bot added the stale label Nov 15, 2023
Copy link

I closed this now because it has been inactive for more than one year. If I closed it by mistake, please do not hesitate to reopen it or create a new issue with the remainder of this issue.
Thank you for your contributions 💖

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants