-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Configure db_definitions key in config
#309
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
Conversation
COUCHDB-2635
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does this define? Why is _dbs plural?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It defines a list of modules which implement couch_define_db behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is plural because couch_define_db behavior defines multiple dbs (see).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, now I get it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we not have this list in config at all?
Here is the problem: you're going to install couch_foo plugin which has couch_define_db behaviour. Plugin install happens in one-single-click way, so user don't have to touch any configs for that. But now, to let this plugin works it need to add himself to that list. That literally means: config:get => put itself these or ensure that it's defined once and only => serialize list => config:set...mmm, ok. Now we want to uninstall that plugin...
You got the idea? Having couch_define_db behaviour isn't enough to let your (abstract) app work correctly: you have to modify this config value. So, technically, we still have a SYSTEM_DATABASES list, not in Erlang code, but in ini file. We cannot define own system databases with it, but enforces to keep this list in actual state for all our apps we have.
P.S. Few edge cases:
- In default.ini we have
[couch_dbs, mem3_dbs, couch_replicator_dbs, cassim_dbs] - User install a couch_foo plugin which puts a new value in local.ini
[couch_dbs, mem3_dbs, couch_replicator_dbs, cassim_dbs, couch_foo] - In new CouchDB release we add new app which also defines own system database, so default.ini changed in the way to have
[couch_dbs, mem3_dbs, couch_replicator_dbs, cassim_dbs, couch_new_app]
Question: how we'll deal with upgrade, since values in local.ini overrides default.ini?
What if I have a non existed module in this list from the plugin I just removed?
What if I have it defined here twice or more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kxepal Where do you propose to store registered plugins? We need some form of persistent storage. Considered options are:
- well known key in application environment.
- Didn't quite work. Due to the requirement that all applications need to be loaded.
- configuration file similar to priv/stats_descriptions.cfg.
- It is awkward to scan a filesystem in startup time
- default.ini
- easiest
None of the options considered support one click install.
I think we have a misconception about the scope. Vendors do compile couchdb locally and include applications they need. They do not use single click install. Their goal is to plug the features they've implemented into couchdb without changing couchdb code. It can be thought of as embedding couchdb into a bigger ecosystem of applications. While you are thinking about people who want to download couchdb and few plugins with additional functionality they need and run it on their servers. Which requires different level of expertise. Making downloadable plugins infrastructure is a much bigger scope in terms of developing, testing and support. I don't think I have resources to do that alone in a reasonable time frame. Besides this might be quite complex and rejected by community on this ground.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to mention that I do work on something bigger than the scope of this PR (see plugerl). However it wouldn't be one-click install either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should use dets for persistent things. ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I agree with you that plugins system itself is a very much more bigger question, but there is no need to think about it too much to have problems I mentioned. You may always assume that couch_mrview and couch_replicator are these plugins (however, only the latter fits this role nicely) which are just available out of the box.
Let's limit our scope of "plugins" by these two apps as we know them well. Both adds own system databases, both could be optional (let's dream a bit here), both vendor may want to replace with own solution (in additional to the others it has).
The biggest problem here is unavailability to gracefully handle CouchDB upgrade when you'd add own db_definitions to local.ini as overlay system demands. If you'd edit them in default.ini you'll end with manual list merge. So if CouchDB ships with none of them, vendor adds own couch_foo, user installs couch_replicator and later CouchDB adds couch_mrview everything gets eventually broken.
This problem actually could be easily solved by turning horizontal list into vertical:
[dbs_definitions]
couch_mrview = couch_mrview_dbs
mem3 = mem3_dbs
Where key is application name and value is a related module. Overlay mechanism will provide all the features to turn on / turn off specific modules with easy.
But still...is there no way to avoid exposing such kind of internal information in config file?
Why application environment case doesn't works? Once application is loaded it publish the module with the behaviour about system databases it needs. If it's not loaded then it's not used.
Can we relay on that if some app wanted to define own system database it mush have specially named module with implemented behaviour?
Maybe we should use dets for persistent things. ;-)
No, let's create another system database which stores other system databases names (; And we need to go deeper (:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why application environment case doesn't works?
Because it relies on a hack which makes testing harder.
Can we relay on that if some app wanted to define own system database it mush have specially named module with implemented behaviour?
Using convention might work but needs to be very well documented and explained.
But still...is there no way to avoid exposing such kind of internal information in config file?
There is also a possibility to discover all modules implementing given behavior using
lists:keyfind(behaviour, 1, <module>:module_info(attributes)). However we would need to iterate through all modules using code:all_loaded. Which gives us the same problem really. Modules have to be loaded.
|
Implemented using different means |
Add typer target (rebase of apache#309)
COUCHDB-2635