Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

[FLOSS H3] yajl: fix silent failure on meta-key insertion #4724

Merged
merged 1 commit into from
Dec 11, 2022

Conversation

Bujuhu
Copy link
Contributor

@Bujuhu Bujuhu commented Nov 29, 2022

fixes #4581

Basics

  • Short descriptions of your changes are in the release notes
    (added as entry in doc/news/_preparation_next_release.md which
    contains _(my name)_)
    Please always add something to the release notes.
  • Details of what you changed are in commit messages
    (first line should have module: short statement syntax)
  • References to issues, e.g. close #X, are in the commit messages.
  • The buildservers are happy. If not, fix in this order:
    • add a line in doc/news/_preparation_next_release.md
    • reformat the code with scripts/dev/reformat-all
    • make all unit tests pass
    • fix all memleaks
  • The PR is rebased with current master.

Checklist

  • I added unit tests for my code
  • I fully described what my PR does in the documentation
    (not in the PR description)
  • I fixed all affected documentation (see Documentation Guidelines)
  • I fixed all affected decisions (see Decision Process)
  • I added code comments, logging, and assertions as appropriate (see Coding Guidelines)
  • I updated all meta data (e.g. README.md of plugins and METADATA.ini)
  • I mentioned every code not directly written by me in reuse syntax

Review

Labels

  • Add the "work in progress" label if you do not want the PR to be reviewed yet.
  • Add the "ready to merge" label if the basics are fulfilled and no further pushes are planned by you.

@Bujuhu Bujuhu force-pushed the yajl/metakeyvalidation branch from f9dc2f3 to 144348f Compare November 29, 2022 16:41
@markus2330
Copy link
Contributor

@kodebach can you maybe help out here? There is a CI error which doesn't have an obvious relation with the code changes here:

[1] Sorry, module kdb issued the warning C01320:
	Interface: Calling the kdbGet function for the backend plugin ('backend') of the mountpoint 'system:/elektra' has failed during the STORAGE phase.
	Mountpoint: system:/elektra
	Configfile: /home/jenkins/workspace/libelektra_PR-4724/config/kdb/system/elektra.ecf
	At: /home/jenkins/workspace/libelektra_PR-4724/src/libs/elektra/kdb.c:1566
[2] Sorry, module kdb issued the warning C01320:
	Interface: The STORAGE phase of kdbGet() has failed. See warnings for details.
	Mountpoint: system:/elektra
	Configfile: /home/jenkins/workspace/libelektra_PR-4724/config/kdb/system/elektra.ecf
	At: /home/jenkins/workspace/libelektra_PR-4724/src/libs/elektra/kdb.c:1585
[3] Sorry, module kdb issued the warning C01100:
	Resource: Could not open configuration file /home/jenkins/workspace/libelektra_PR-4724/config/kdb/system/elektra.ecf for reading. Reason: No such file or directory
	Mountpoint: /
	Configfile: 
	At: /home/jenkins/workspace/libelektra_PR-4724/src/libs/elektra/errors.c:189
Sorry, module kdb issued the error C01200:
Installation: Bootstrapping failed, please fix '/home/jenkins/workspace/libelektra_PR-4724/config/kdb/system/elektra.ecf'. If the error persists, please report this bug at https://issues.libelektra.org.
Mountpoint: /
Configfile: 
At: /home/jenkins/workspace/libelektra_PR-4724/src/libs/elektra/kdb.c:380

@kodebach
Copy link
Member

kodebach commented Dec 5, 2022

This is another example of #4671 (I added it). I still believe rerun-failed is a mistake. However, here only 1 test failed.

I believe what happen is:

  1. Some random event caused /home/jenkins/workspace/libelektra_PR-4724/config/kdb/system/elektra.ecf to not exist when testcpp_kdb expected it to be there.
  2. testcpp_kdb failed
  3. Jenkins (more specifically the CTest Dashboard stuff) detected and remembered that
  4. The rerun-failed run tries testcpp_kdb again
  5. This time the random event does not happen and testcpp_kdb succeeds. Therefore, the pipeline does not abort, but we still see a failed test.

The solution IMO: just rerun Jenkins.

I have yet to see an example where rerun-failed actually helped. So far I've only see cases like this, where everybody is confused and we end up doing a manual rerun anyway...

@kodebach
Copy link
Member

kodebach commented Dec 5, 2022

Jenkins build libelektra please

@deoknats861
Copy link
Contributor

Review

Documentation
The notes added for the upcoming release describe everything what the PR does clearly.

Examples are well chosen and understandable
The added test covers the added functionality and is understandable.

Code is conforming to our Coding Guidelines
Yes.

APIs are conforming to our Design Guidelines
The API was not changed.

Code is consistent to our Design Decisions
Yes.

@flo91
Copy link
Collaborator

flo91 commented Dec 11, 2022

Thank you! 🎉

@flo91 flo91 merged commit 8aff565 into ElektraInitiative:master Dec 11, 2022
@mpranj mpranj added this to the 0.9.12 milestone Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FLOSS] yajl plugin fails silently on meta-set
6 participants