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

Different set of config data passed from NuleculeBase config to providers. #331

Open
cdrage opened this issue Oct 16, 2015 · 8 comments
Open
Assignees

Comments

@cdrage
Copy link
Member

cdrage commented Oct 16, 2015

atomicapp master:

2015-10-16 10:23:21,606 - atomicapp.nulecule_base - DEBUG - Data given {'general': {'namespace': 'test', 'provider': 'docker'}}
2015-10-16 10:23:21,606 - atomicapp.plugin - DEBUG - Found provider <class 'docker.DockerProvider'>
2015-10-16 10:23:21,607 - atomicapp.plugin - WARNING - Configuration option 'providerconfig' not found
2015-10-16 10:23:21,607 - docker - DEBUG - Given config: {'namespace': 'test', 'provider': 'docker'}

cdk2-beta3-rc1:

- WARNING - Configuration option 'providerconfig' not found
2015-10-16 10:32:10,396 - atomicapp.providers.docker - DEBUG - Given config: {'general': {'namespace': 'test', 'provider': 'docker'}}
2015-10-16 10:32:10,396 - atomicapp.providers.docker - DEBUG - Namespace: default

See Given config: line.

Now... for future integration and more information, I believe we should pass the entire config (including "general"), which is what cdk2-beta3-rc1 is doing right now. If we perhaps happen to include more variables in the future, we wouldn't want to do another PR to fix and perhaps break integration.

This will however change the:
self.namespace = self.config.get("namespace")
to
self.namespace = self.config["general"]["namespace"]

In all providers.

@dustymabe
Copy link
Contributor

@rtnpro - I'm interested in your take on this.

@cdrage
Copy link
Member Author

cdrage commented Oct 16, 2015

UPDATE

Seems it was a testing error for configuration. Either way, still want to discuss whether we want to pass the entire configuration to provider.

@rtnpro
Copy link
Contributor

rtnpro commented Oct 16, 2015

@charliedrage
Can we use get_context() to supply normalized config data to a provider?
Usage: https://github.com/projectatomic/atomicapp/blob/cdk2-beta3-rc1/atomicapp/nulecule/base.py#L276
Code: https://github.com/projectatomic/atomicapp/blob/cdk2-beta3-rc1/atomicapp/nulecule/lib.py#L49

Will the above solve this issue?

@cdrage
Copy link
Member Author

cdrage commented Oct 16, 2015

@rtnpro Yup, that will.

We will however have to change the Nulecule spec to conform to this. We should label this issue for after GA / Beta-3.

@rtnpro
Copy link
Contributor

rtnpro commented Oct 16, 2015

Agreed 👍

@dustymabe dustymabe added this to the CDK 2 GA milestone Oct 16, 2015
@dustymabe
Copy link
Contributor

@cdrage @rtnpro has the goal of this issue changed at all. I know there is #423 which might affect this.

@cdrage
Copy link
Member Author

cdrage commented Dec 3, 2015

@dustymabe With the persistent storage implementation we will need a way to pass persistent storage information to the provider (if we go the way of having a def persistent_storage function within the provider and the provider handling it). So yes, this still needs to be implemented.

@cdrage
Copy link
Member Author

cdrage commented Jan 26, 2016

Hey @rtnpro you are working on a Config class for this. Seems that this issue would be fixed upon the completion of it, I'm going to assign this to you for now. :)

@cdrage cdrage assigned rtnpro and unassigned cdrage Jan 26, 2016
@rtnpro rtnpro removed this from the CDK 2 GA milestone Feb 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants