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

remove internal meta-data #3020

Closed
markus2330 opened this issue Sep 30, 2019 · 6 comments
Closed

remove internal meta-data #3020

markus2330 opened this issue Sep 30, 2019 · 6 comments

Comments

@markus2330
Copy link
Contributor

@kodebach reported in #1164:

Other problems with kdb set are:

  • internal/** metadata is not removed
  • origvalue metadata is not removed (breaks all future kdb get, until it is removed)

fixing it in kdb is quite straight-forward. But isn't it better to fix it in the spec plugin, then not every tool needs to reimplement it?

See also #2700 (currently open because the global position for spec does not work).

@kodebach
Copy link
Member

But isn't it better to fix it in the spec plugin

The spec plugin is not the problem in this case. origvalue shouldn't exist in the spec namespace, so the spec plugin won't copy it. internal/** is already ignored by spec (although it shouldn't need to do this IMO). In my case it was the internal/ini/** metadata (created by the ini plugin) that got stored.

A plugin running after spec could also still add internal/** or the origvalue metadata.

Note: I actually have no idea, where the origvalue metadata comes from. The type plugin calls keySetString, which in turn always removes origvalue. The only possible explanation I have right now, is that the type metadata was removed between kdbGet and kdbSet (if so, not intentionally).

fixing it in kdb is quite straight-forward.

Fixing it in kdb is actually impossible. The tool cannot influence, which plugins are called. Therefore it cannot prevent plugins from adding such metadata. AFAIK kdb does everything correctly.

IMO there has to be a way to mark metakeys (and maybe even normal keys) for automatic removal. All the marked keys should then be automatically removed, before the storage plugin is called.

@markus2330
Copy link
Contributor Author

The spec plugin is not the problem in this case. [...] A plugin running after spec could also still add internal/** or the origvalue metadata.

We once discussed that spec should be the vacuum cleaner to fix everything that is wrong with metadata.

A plugin running after spec could also still add internal/** or the origvalue metadata.

The idea was that spec will be the very last plugin.

Fixing it in kdb is actually impossible.

Yes, if the metadata is added during kdbSet. I thought we talk about metadata that should vanish between kdbGet and before kdbSet.

IMO there has to be a way to mark metakeys (and maybe even normal keys) for automatic removal. All the marked keys should then be automatically removed, before the storage plugin is called.

There is no need for automatic removal because the semantics as given in doc/METADATA.ini already tell us what should be removed and what is needed for storage plugins. The spec plugin would need a list of metadata to be removed. This is not super-pretty but the alternatives need code-generation which is too much for 1.0 now.

@kodebach
Copy link
Member

We once discussed that spec should be the vacuum cleaner to fix everything that is wrong with metadata.
The idea was that spec will be the very last plugin.

Yes, I know (#2700 (comment)), but it is not possible right now. I also think spec is not the right plugin for that.

If we don't want to do the cleanup in the core (or the new backend plugin), it should be a separate cleanup plugin. spec is already complicated enough.

I thought we talk about metadata that should vanish between kdbGet and before kdbSet.

I don't know, where the internal/** metadata should vanish. That depends on the metadata. origvalue should definitely not vanish between kdbGet and kdbSet (unless the key was changed) that would defeat its whole point.

There is no need for automatic removal because the semantics as given in doc/METADATA.ini already tell us what should be removed and what is needed for storage plugins. The spec plugin would need a list of metadata to be removed. This is not super-pretty but the alternatives need code-generation which is too much for 1.0 now.

It might be true that doc/METADATA.ini tells us what should be removed and what shouldn't (although not in an explicit machine-readable way), but marking (meta-)keys that should be removed would be a lot easier.

@markus2330
Copy link
Contributor Author

If we don't want to do the cleanup in the core (or the new backend plugin), it should be a separate cleanup plugin. spec is already complicated enough.

Fair point. We can also make a new cleanup plugin. I thought that having it in spec is okay because the cleanup code in spec is not complicated.

origvalue should definitely not vanish between kdbGet and kdbSet (unless the key was changed) that would defeat its whole point.

Nothing should vanish between kdbGet and kdbSet, see explanation in #2700. The crucial point here is to implement the hook correctly. Afaik it was never correct and because of that it never worked.

but marking (meta-)keys that should be removed would be a lot easier.

For the removal code: certainly. Overall: I do not know. There are many plugins that set metadata to be removed, they all would need to be changed to mark their metadata.

Furthermore, this would open the door for many different markers. E.g. for storage plugins we might want to remove different meta-data as for exporting configuration.

@stale
Copy link

stale bot commented Sep 29, 2020

I mark this issue 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 the issue by writing a message here or create a new issue with the remainder of this issue.
Thank you for your contributions 💖

@stale stale bot added the stale label Sep 29, 2020
@stale
Copy link

stale bot commented Oct 13, 2020

I closed this issue 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 💖

@stale stale bot closed this as completed Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants