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

proposal: kdbEnsure #868

Closed
markus2330 opened this issue Jul 29, 2016 · 18 comments
Closed

proposal: kdbEnsure #868

markus2330 opened this issue Jul 29, 2016 · 18 comments
Labels
Milestone

Comments

@markus2330
Copy link
Contributor

As discussed we need for some applications a way to ensure that some global plugin are present. For example, e.g. applications that need the cachefilter (written by @Namoshek) or need the internalnotification ( #828 written by @waht).

More generally put, some Applications need behaviour from Elektra that was not there some versions ago and cannot be the new default.

In the long term, we should extend kdbOpen so that users can request any functionality (either by plugins or the core itself).

In the shorter term, we could add a kdbEnsure that will add a global plugin if it was not there before, exactly fulfilling the current needs.

A possible API would be:

int kdbEnsure (KDB *kdb, KeySet *contractToEnsure, Key *errorKey);
@Namoshek
Copy link
Contributor

kdbEnsure sounds like a good way to go about it for now. To me changing kdbOpen looks like a deep change that would probably fit more during a new major version of the library where breaking changes are fine. I know it is possible to overload functions with some tricks since C11, but I'd rather not deploy such a big change with a minor version.

@markus2330
Copy link
Contributor Author

The current plan is that we create new APIs (similar to keyRel2 #826) in libproposal, later we can use symbol versioning like libc does, and with 1.0.0 we get rid of the legacy stuff (maybe move it to libelektra-legacy).

@markus2330
Copy link
Contributor Author

@kodebach Is this maybe a byproduct of the checks done within the high-level API?

@markus2330
Copy link
Contributor Author

In #2383 we found that sometimes the absence of plugins might be required (e.g. for cmd-line options).

So for now, we either want to say that a global plugin needs to be present (internalnotification or also cmd-line options) or should be absent (cmd-line options for maintenance tasks).

@kodebach
Copy link
Member

Instead of creating kdbEnsure, we could also pass the information to kdbOpen or kdbGet directly.

The advantage of passing the information to kdbOpen directly is that we might be able to avoid doing unnecessary work, which would only be undone by kdbEnsure. However, the only way of passing information to kdbOpen would be using the metadata of the errorKey parameter. This seems kind of inconvenient compared to passing an actual KeySet. One possibility would be to add kdbOpen2 to libproposal as mentioned above.

kdbGet meanwhile already receives a KeySet, so we could simply use keys below proc/elektra (so they won't interfere with actual keys retrieved by kdbGet). The only real advantage here is that no extra function call and therefore no extra error handling is needed.

@markus2330 markus2330 added this to the 0.9.0 milestone Feb 22, 2019
@markus2330
Copy link
Contributor Author

Yes, modifying kdbOpen is the long-term solution.

Maybe we can even break the ABI soon and release 0.9.

@kodebach
Copy link
Member

We could do something similar to keySetName and elektraKeySetName.

@markus2330
Copy link
Contributor Author

Yes, we could add elektraKdbOpen in libproposal.

But it is slowly getting messy.

@markus2330
Copy link
Contributor Author

My next step will be adding kdbEnsure. However, to do that I need to know, what functionality it should support and how generic the contract should be?

The contract can be very generic but mounting/unmounting of plugins can still be the only feature for now. I am not sure if only global plugins suffice, as some application developers might also consider non-global plugins essential for their application (e.g. profile)? And didn't we even say that the gopts plugin will be non-global? If we do not need non-global plugins for now, only mounting/unmounting of global plugins is enough.

At least for non-global plugins (but maybe also global plugins?) we should ask ourselves the question if we really want to mount/unmount plugins or is it better to let the application fail if the mounting is done wrongly. (The plugin config kdbEnsure delives might be unwanted.)

Should it only support mounting/unmountig global plugins or should it also replace the notification library? (Which AFAIK is only there to setup the internalnotification plugin)

Imho the interface for the notification library is quite nice. Do you see here any room for improvement?

If kdbEnsure should be able to do more than mounting/unmounting, I think we need some interface to the plugins, so the plugin specific code stays isolated...

Having good cohesion is nice, independent of the feature set we implement.

@kodebach
Copy link
Member

And didn't we even say that the gopts plugin will be non-global?

It turns out a global plugin makes the interaction easier, even if the plugin doesn't need to know about the whole KDB. Global plugins always receive the parent key given to kdbGet, which is exactly what we want for gopts.

but maybe also global plugins?

Because of the kdb tool, there has to be at least the possibility to disable gopts.

The plugin config kdbEnsure delives might be unwanted.

My idea was either to fail, if the given mounted requirements are not fulfilled, or to require a full configuration to be given when a plugin is required to be mounted. The idea basically being either the user mounts the plugin themselves with their own configuration, or the default from inside the application is used.

Unmounting/disabling a plugin should always work

Imho the interface for the notification library is quite nice. Do you see here any room for improvement?

The interface is definitely nice, but in principle what the library does, is basically the intended use case for kdbEnsure. It also uses kdbprivate, which isn't really mentioned anywhere (at least not that I have seen).

@markus2330
Copy link
Contributor Author

Because of the kdb tool, there has to be at least the possibility to disable gopts.

And also to enable for applications that rely on cmd-line options.

or to require a full configuration to be given when a plugin is required to be mounted

Yes, sounds very good.

but in principle what the library does, is basically the intended use case for kdbEnsure

Yes, the you are right. With KEY_FUNC passing the pointer to the main-loop might be not so bad after all. I hope this change will not be a Pandora's box.

It also uses kdbprivate, which isn't really mentioned anywhere (at least not that I have seen).

Really? In a file included by users? That really should not happen 😢

@kodebach
Copy link
Member

I hope this change will not be a Pandora's box.

There is definitely a trade-off between having strongly typed functions and function pointer and using a generic KeySet. We might want to leave the library for notifications (they are quite a special case) and just invoke kdbEnsure in there to mount the plugin. This should get rid of the dependency on kdbprivate as well. But it would avoid KEY_FUNC.

Maybe this could even be the solution in general. kdbEnsure is used to ensure certain plugins are mounted/unmounted and a second function (kdbFindPlugin or similar) is used to obtain a handle for a certain plugin that can then be used to call any configuration functions that a plugin provides (via elektraInvokeGetFunction).

In a file included by users?

It is only included in the C file, so users won't notice it. It still requires that the version of libnotification and elektra-core and elektra-kdb are compatible though.

@kodebach
Copy link
Member

The implementation in #2471 right now supports mounting/unmounting global plugins, as well as unmounting non-global plugins (mounting them is quite a bit more complicated). I am thinking about allowing to add a new mountpoint with only a storage plugin

More functionality shouldn't be added at the moment IMO, because the plugin framework is going to change.

I also noticed that in some sense a fully functional kdbEnsure is a re-implementation of the BackendBuilder class et al., because ideally kdbEnsure would know about the infos/needs and infos/recommends metadata. It also should know about infos/ordering (one of the reasons I didn't implement mounting for non-global plugins).
So IMO we should either rewrite the tools library in C and use it in kdbEnsure, or we build the full kdbEnsure and provide a function that persists the changes made by kdbEnsure into a KeySet.

@markus2330
Copy link
Contributor Author

Yes, the tools library should be rewritten in C (or Rust) but actually I would like to avoid to have it pulled into kdb itself.

@kodebach
Copy link
Member

the tools library should be rewritten in C (or Rust)

Wouldn't Rust cause the same problems as C++? i.e. requiring Rust standard libraries on any system using it (unless you use Rust's #![no_std] feature).

@markus2330
Copy link
Contributor Author

Maybe, I did not yet compare the size of the run-time libs of C++/Rust.

@kodebach
Copy link
Member

IMO it is not just about size but also about availability. Rust does support a lot of systems, but C certainly supports more. It would be very weird to have a library like elektra-kdb that is written in C depend on Rust, just for one function (kdbEnsure) and thereby limiting the number of supported systems.

@markus2330
Copy link
Contributor Author

Yes, of course. it will be a long way for any language to be as widely available as C.

just for one function

I was talking about rewriting libtools. I agree: Only for kdbEnsure it would be strange.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants