Skip to content
This repository has been archived by the owner on Jan 19, 2018. It is now read-only.

Refactor Nulecule params #442

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

rtnpro
Copy link
Contributor

@rtnpro rtnpro commented Dec 7, 2015

Refactor Nulecule/atomicapp params to support developing nested Nulecule applications easy. This work is to fix the issues discussed in projectatomic/nulecule#187

Fixes #423.

Fixes projectatomic#423.

Date:      Fri Dec 04 15:30:39 2015 +0530
- bugfix for not updating values for params in config with static value.
@rtnpro
Copy link
Contributor Author

rtnpro commented Dec 24, 2015

@kanarip @dustymabe @vpavlin @cdrage
You can try out this code with projectatomic/nulecule-library#30

@cdrage
Copy link
Member

cdrage commented Dec 24, 2015

@rtnpro a lot of tests fail, we'll have to have compatibility from putting params both under graph and params, unless our spec changes in the future

@dustymabe
Copy link
Contributor

I have not looked at the code at all but have looked at the POC code in projectatomic/nulecule-library#30. This looks interesting and I am going through and going to try to convert my etherpad app to do the same thing and see what all I need to change.

@cdrage. Do you mind looking at this in detail and also consider the work you did for xpathing when you review? Start with projectatomic/nulecule-library#30 and then we can both look at the code in this PR.

@dustymabe
Copy link
Contributor

One thing I noticed when playing around with this is that only parts of the config are getting passed to the provider. An example is that if I put 'providerconfig' into the answers.conf file it does not make it into the provider code. The logger.debug("Given config: %s", self.config) from the kubernetes provider is a good one to look at to see what is getting passed.

@rtnpro
Copy link
Contributor Author

rtnpro commented Jan 15, 2016

@vpavlin @dustymabe @cdrage @kadel @goern

So, I updated the POC for refactoring params in Nulecule spec here:
rtnpro/nulecule-library@4c9ee7b

It allows:

Cross referencing params

This allows referencing params definition across sibling components in a Nulecule. It fixes duplicate param problem in answers or config data.

Config mapping

It allows channelize config from current Nulecule to its external Nulecule children, or, local component according to their needs. This allows, optionally, in controlling access to config data to external Nulecule apps.

The end result will be a cleaner and understandable answers file, where users enter
param values only for the immediate Nulecule they are consuming.

However, this will not take the freedom of overriding config data for a Nulecule component at any level in the Nulecule tree by adding a section for it in answers file, just like the way it works now.

@dustymabe
Copy link
Contributor

@rtnpro I looked at your sentry example and it seems like there is a lot of mixing of ideas in there, which is probably a mistake. Do you mind adding your new proposal to https://gist.github.com/rtnpro/17add2ab248ac34a7687 so it can clearly be evaluated?

@cdrage
Copy link
Member

cdrage commented Mar 17, 2016

Woo! projectatomic/nulecule#196 has been merged, no more blockers @rtnpro

@rtnpro
Copy link
Contributor Author

rtnpro commented Apr 18, 2016

@dustymabe @cdrage @surajssd @containscafeine
Let's resume discussions on this and reach a decision ASAP.

https://gist.github.com/rtnpro/17add2ab248ac34a7687#file-nulecule_with_param_cross_ref_and__optional_mapping-md

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

Successfully merging this pull request may close these issues.

Refactor ANSWERS to improve support for nested Nulecule applications
3 participants