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

dini: new default storage plugin #1693

Closed
markus2330 opened this Issue Nov 2, 2017 · 6 comments

Comments

3 participants
@markus2330
Contributor

markus2330 commented Nov 2, 2017

To be done

  • add binary plugin as dep
  • #1713 ASAN
  • testshell_markdown_dini fails
  • cannot write new dini files (conflict)

Specification

For the migration from dump to ini, we need a new plugin dini (for "default ini" or "dump ini"), which is capable of:

  • reading (but not writing!) dump files with the dump plugin, if it fails reading a dump file it falls back to:
  • reading and writing ini files with the ini plugin
  • instantiate plugins the ini plugin needs (null, binary, dir2leaf #106, others?)

Every error that happens during reading with dump is ignored, including the absence of the dump plugin. This allows the dump plugin to be dropped as hard dependency. (e.g. people not caring about migration but caring about the plugins ini needs). People might even remove the dump plugin on purpose because it obviously causes some overhead.

The goals of the plugin are:

  • to be completely compatible with Elektra installations that currently use dump
  • safely migrate such installations to use INI
  • to be able to represent every KeySet, like dump did (thus null, binary, dir2leaf, etc. plugins might be needed)

When we have dini, we can make the plugin dump obsolete. dump then only is used in the context of dini: to migrate legacy Elektra installations.

What do you think?

@sanssecours

This comment has been minimized.

Show comment
Hide comment
@sanssecours

sanssecours Nov 9, 2017

Contributor

What do you think?

Sounds good to me (as long as I do not have to implement the dini plugin ☺️).

Contributor

sanssecours commented Nov 9, 2017

What do you think?

Sounds good to me (as long as I do not have to implement the dini plugin ☺️).

@sanssecours sanssecours removed their assignment Nov 9, 2017

@markus2330 markus2330 assigned markus2330 and unassigned tom-wa Nov 24, 2017

@markus2330 markus2330 referenced this issue Dec 6, 2017

Closed

dini: Plugin Breaks Test Suite #1713

3 of 3 tasks complete
@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Dec 10, 2017

Contributor

Are there any further leftovers for dini except merging #1722?

To everyone: Please use and test the dini plugin.

Contributor

markus2330 commented Dec 10, 2017

Are there any further leftovers for dini except merging #1722?

To everyone: Please use and test the dini plugin.

@sanssecours

This comment has been minimized.

Show comment
Hide comment
@sanssecours

sanssecours Dec 12, 2017

Contributor

Are there any further leftovers for dini except merging #1722?

I do not think so. I already pulled the relevant changes from #1722 into the master branch yesterday.

To everyone: Please use and test the dini plugin.

I currently use dini as default storage. As far as I can tell everything works fine 🙌.

Contributor

sanssecours commented Dec 12, 2017

Are there any further leftovers for dini except merging #1722?

I do not think so. I already pulled the relevant changes from #1722 into the master branch yesterday.

To everyone: Please use and test the dini plugin.

I currently use dini as default storage. As far as I can tell everything works fine 🙌.

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Dec 12, 2017

Contributor

In particular we should test if partly-done-migrations (like /etc/kdb/default.ecf is already INI but /etc/kdb/elektra.ecf is not) causes any problems.

Do we need (or can we) add other plugins to be used within INI? (null, binary, directoryvalue?)

Contributor

markus2330 commented Dec 12, 2017

In particular we should test if partly-done-migrations (like /etc/kdb/default.ecf is already INI but /etc/kdb/elektra.ecf is not) causes any problems.

Do we need (or can we) add other plugins to be used within INI? (null, binary, directoryvalue?)

@sanssecours

This comment has been minimized.

Show comment
Hide comment
@sanssecours

sanssecours Dec 13, 2017

Contributor

Do we need (or can we) add other plugins to be used within INI? (null, binary, directoryvalue?)

  • Adding directoryvalue to INI does not work, as already stated here.

  • Fortunately using Base64 (binary) together with INI seems to work. I already added a Markdown Shell Recorder test for this situation in one of the branches of my repo.

  • I do not think it makes much sense to also use the null plugin together with INI, since INI already supports empty values. If we use Base64, then INI can also save empty binary values.

Contributor

sanssecours commented Dec 13, 2017

Do we need (or can we) add other plugins to be used within INI? (null, binary, directoryvalue?)

  • Adding directoryvalue to INI does not work, as already stated here.

  • Fortunately using Base64 (binary) together with INI seems to work. I already added a Markdown Shell Recorder test for this situation in one of the branches of my repo.

  • I do not think it makes much sense to also use the null plugin together with INI, since INI already supports empty values. If we use Base64, then INI can also save empty binary values.

@sanssecours sanssecours referenced this issue Dec 13, 2017

Closed

Release 0.8.21 #1725

12 of 12 tasks complete

@markus2330 markus2330 removed the proposal label Dec 13, 2017

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Dec 13, 2017

Contributor

Thank you, that is great, then let us add binary/base64. (I added it in the top post)

Contributor

markus2330 commented Dec 13, 2017

Thank you, that is great, then let us add binary/base64. (I added it in the top post)

@markus2330 markus2330 referenced this issue Dec 19, 2017

Merged

Kdb mount #1745

3 of 7 tasks complete

@markus2330 markus2330 added this to not needed in lcdproc Mar 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment