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] spec: reimplement to completely comply with hooks #4500

Open
atmaxinger opened this issue Sep 29, 2022 · 5 comments
Open

[new-backend] spec: reimplement to completely comply with hooks #4500

atmaxinger opened this issue Sep 29, 2022 · 5 comments
Assignees
Labels

Comments

@atmaxinger
Copy link
Contributor

atmaxinger commented Sep 29, 2022

The spec plugin isn't completely conformant with what should be done in the spec hook. When implementig the hook, we decided to not waste too much time on it, just that the basisc work.


There can be different settings for the conflict handling

  • This can probably be removed

Yes, all the conflict handling settings should be removed entirely. During get all conflicts should be warnings during set they should be errors (unless @markus2330 thinks some conflicts should never be errors). That part could be handled from the libelektra-kdb side with the "error blocking trick", so the spec would be the same.

neither assign/condition nor default are processed in set

Not sure about assign/condition and whether we actually want to keep that (@markus2330 ?), but default should definitely be processed in set. See also #4484 (comment)

  • set operation sets internal/spec/remove on array
  • set removes internal/spec/array/validated
  • set does this thing here, I'm not sure what it is

All of the stuff related to array handling is very complicated and weird. Part of the weirdness comes from the fact that on master the spec plugin is only called once during kdbSet and somehow there should still be validation during kdbSet, but also spec should cleanup the copied data (obviously not possible).

See also #2700 and #4061


However, please don't waste time on this. Like I said already: Just try to imitate what spec does on master. The existing tests obviously also expect this behaviour. Once that is done and merged, you can create another PR with the actual updates for spec (probably after new-backend is merged in master).

Originally posted by @kodebach in #4495 (comment)

@atmaxinger atmaxinger self-assigned this Sep 29, 2022
@atmaxinger atmaxinger mentioned this issue Sep 29, 2022
26 tasks
@kodebach
Copy link
Member

Another important thing is that default:/ keys should be processed in the poststorage phase of kdbGet, currently the are temporarily cut out during that phase.

@atmaxinger
Copy link
Contributor Author

Yes - See also #4493

@markus2330
Copy link
Contributor

@atmaxinger is this done?

@atmaxinger
Copy link
Contributor Author

Well as @tmakar has rewritten the spec plugin, I think this is done.

Copy link

github-actions bot commented May 2, 2024

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 May 2, 2024
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

3 participants