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

kdb export metadata #2738

Open
kodebach opened this issue Jun 1, 2019 · 5 comments

Comments

Projects
None yet
4 participants
@kodebach
Copy link
Contributor

commented Jun 1, 2019

kdb export doesn't remove internal/* metadata. It also doesn't respect the special origvalue metakey. The reason for this is that it calls the full kdbGet, but only the set method of the storage plugin.

There are three solutions to this:

  1. Don't call the full kdbGet. Instead just find the mountpoint and call the get function of the current storage plugin for that mountpoint directly. The call the set function of the new storage plugin directly (like before). This way we ensure that only the stored data is converted to the new format. It however also circumvents all validation plugin, so the exported data might not be valid (if files were modified manually).

  2. Construct a new backend from the current one by replacing the storage plugin. Then use that backend to export the data. Because of the way kdbSet works, I'm not sure how easy it is to do that, however. We would probably have to mount the backend fully, copy the data over, print out the file and then unmount the backend.

  3. Find the current backend and manually call all the setstorage plugins. This is not a good solution IMO, because we then need to keep kdb export in sync with kdb set (e.g. if we introduce new positions).

Personally, I think the first solution is the best, but circumventing all validation plugins might cause problems.


NOTE: I didn't provide steps for reproduction, even though this is a bug report. The bug is obvious from the code:

int ExportCommand::execute (Cmdline const & cl)
{
size_t argc = cl.arguments.size ();
if (argc != 1 && argc != 2 && argc != 3)
{
throw invalid_argument ("need 1 to 3 arguments");
}
Key root = cl.createKey (0);
kdb.get (ks, root);
printWarnings (cerr, root, cl.verbose, cl.debug);
KeySet part (ks.cut (root));
if (cl.withoutElektra)
{
Key systemElektra ("system/elektra", KEY_END);
part.cut (systemElektra);
}
string format = cl.format;
if (argc > 1) format = cl.arguments[1];
#ifdef _WIN32
string file = "CON";
#else
string file = "/dev/stdout";
#endif
if (argc > 2 && cl.arguments[2] != "-") file = cl.arguments[2];
Modules modules;
PluginPtr plugin = modules.load (format, cl.getPluginsConfig ());
Key errorKey (root);
errorKey.setString (file);
plugin->set (part, errorKey);
printWarnings (cerr, errorKey, cl.verbose, cl.debug);
printError (cerr, errorKey, cl.verbose, cl.debug);
return 0;
}

@kodebach kodebach added the bug label Jun 1, 2019

@mpranj

This comment has been minimized.

Copy link
Member

commented Jun 1, 2019

Wouldn't dropping internal/* break ini's ordering when doing export and then import, since ini relies on this? Otherwise this "fixes" the problem I've had with caching ini's data.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2019

Thank you for reporting the bug!

kdb export doesn't remove internal/* metadata

Yes, this is a bug. It should be removed by post-processing the KeySet.

It also doesn't respect the special origvalue metakey.

This is a quite new idea (proposed in #2724). Yes, it also makes sense for kdb export.

It also could be done easily in postprocessing.

The reason for this is that it calls the full kdbGet, but only the set method of the storage plugin.

Even if you would call a full kdbGet, you might need to postprocess some metadata as the storage plugin used for exporting might not support internal or origvalue.

I didn't provide steps for reproduction, even though this is a bug report. The bug is obvious from the code:

Bugs are always obvious from the code once you understand the code. Nevertheless showing the wanted behavior is important. Otherwise the new implementation might still not do what you expected.

@llukask hopefully will address these problems.

Wouldn't dropping internal/* break ini's ordering

Yes, it would. But this is a bug of INI as it actually should use "order" and not "internal/order".

Otherwise this "fixes" the problem I've had with caching ini's data.

Didn't removing the meta-data of the parentKey also fix the problem?

@mpranj

This comment has been minimized.

Copy link
Member

commented Jun 1, 2019

Didn't removing the meta-data of the parentKey also fix the problem?

Simply removing it didn't help and it really broke the ordering.

@kodebach

This comment has been minimized.

Copy link
Contributor Author

commented Jun 1, 2019

Wouldn't dropping internal/* break ini's ordering when doing export and then import, since ini relies on this?

Yes, but I didn't actually suggest dropping any metadata in kdb export. My 3 suggestions produce less metadata (1) and/or call the set functions of the plugins that produced the metadata (1,2,3). Plugins are required to remove all their internal/* metadata in their set function, so this solves the problem. The same is true for origvalue.

It also could be done easily in postprocessing.

I don't think post-processing is a good solution. It requires that kdb export knows about every special behaviour of each storage plugin, which is not the point of a plugin system.

Yes, it would. But this is a bug of INI as it actually should use "order" and not "internal/order".

Actually it should use internal/ini/order, unless we generally support the order functionality. Then it would be possible to drop all internal/* metadata, except for that of the storage plugin we are calling.

Otherwise that would mean storage plugins are not allowed to use any internal/* metadata. And every metakey used in a storage plugin would have to be supported by kdb export.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2019

Plugins are required to remove all their internal/* metadata in their set function, so this solves the problem.

The export plugin might be a different one than the one that is used within kdbGet.

It requires that kdb export knows about every special behaviour of each storage plugin, which is not the point of a plugin system.

The idea would be that doc/METADATA.md defines the behavior.

Actually it should use internal/ini/order, unless we generally support the order functionality.

There is the "order" metadata, only INI does not use it. (And now causes many problems.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.