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

Conversation

@rtnpro
Copy link
Contributor

@rtnpro rtnpro commented Jan 19, 2016

Fixes #378

@dustymabe
Copy link
Contributor

So I don't really like how we are spreading out the integration of the config in different areas but you added unit tests so when we make changes in the future we should catch any regressions we make.

I think in the coming time we will do a complete rework of the handling of the config. Until that time this LGTM.

@cdrage. can you run this through your functional tests?

@rtnpro
Copy link
Contributor Author

rtnpro commented Jan 19, 2016

@dustymabe

As discussed, in the long run, we need to have a Config object rather than a dict to handle config data.

@rtnpro rtnpro force-pushed the defaultprovider-in-global-params branch from c1adc47 to e0f2970 Compare January 19, 2016 16:54
@cdrage
Copy link
Member

cdrage commented Jan 19, 2016

Code LGTM. Tests pass too 👍

Lot's of tests have been added too ✨

dustymabe added a commit that referenced this pull request Jan 19, 2016
Support specifying default provider in Nulecule spec file.
@dustymabe dustymabe merged commit 291b831 into projectatomic:master Jan 19, 2016
@cdrage
Copy link
Member

cdrage commented Jan 19, 2016

Okay, thought it wasn't working, but I was just getting a weird anymarkup yaml error due to random whitespacing being added.

~/test                                                                                                                        
▶ docker build -t test . && sudo atomicapp run test
Sending build context to Docker daemon 38.91 kB
Step 0 : FROM atomicapp:build
 ---> dab9752a306d
Step 1 : MAINTAINER Red Hat, Inc. <container-tools@redhat.com>
 ---> Using cache
 ---> 6a0c6ac97203
Step 2 : LABEL io.projectatomic.nulecule.providers "kubernetes,docker" io.projectatomic.nulecule.specversion "0.0.2"
 ---> Using cache
 ---> 36eb3a8400ff
Step 3 : ADD /Nulecule /Dockerfile README.md /application-entity/
 ---> a13e19a7fa53
Removing intermediate container b4f09cd8581a
Step 4 : ADD /artifacts /application-entity/artifacts
 ---> 7a999e021ff2
Removing intermediate container a47b25c2c4b9
Successfully built 7a999e021ff2
2016-01-19 14:45:40,055 - atomicapp.cli.main - INFO - Action/Mode Selected is: run
2016-01-19 14:45:40,055 - atomicapp.nulecule.base - INFO - Unpacking image: test to /var/lib/atomicapp/test-17e6ca5c77b9
2016-01-19 14:45:42,809 - atomicapp.nulecule.container - INFO - Skipping pulling Docker image: test
2016-01-19 14:45:42,809 - atomicapp.nulecule.container - INFO - Extracting nulecule data from image: test to /var/lib/atomicapp/test-17e6ca5c77b9
119b519d6458aa37da062578421e86718c7826027aaebd2ef272a52e69123a1f
2016-01-19 14:45:43,159 - atomicapp.cli.main - ERROR - AnyMarkupError: caught <class 'yaml.scanner.ScannerError'>: mapping values are not allowed here
  in "<file>", line 17, column 13
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/atomicapp-0.3.1-py2.7.egg/atomicapp/cli/main.py", line 100, in cli_run
    nm.run(**argdict)
  File "/usr/local/lib/python2.7/dist-packages/atomicapp-0.3.1-py2.7.egg/atomicapp/nulecule/main.py", line 221, in run
    self.nulecule = self.unpack(dryrun=dryrun, config=self.answers)
  File "/usr/local/lib/python2.7/dist-packages/atomicapp-0.3.1-py2.7.egg/atomicapp/nulecule/main.py", line 126, in unpack
    nodeps=nodeps, dryrun=dryrun, update=update)
  File "/usr/local/lib/python2.7/dist-packages/atomicapp-0.3.1-py2.7.egg/atomicapp/nulecule/base.py", line 105, in unpack
    dryrun=dryrun, update=update)
  File "/usr/local/lib/python2.7/dist-packages/atomicapp-0.3.1-py2.7.egg/atomicapp/nulecule/base.py", line 130, in load_from_path
    nulecule_data = anymarkup.parse_file(nulecule_path)
  File "build/bdist.linux-x86_64/egg/anymarkup_core/__init__.py", line 114, in parse_file
    return parse(f, format, encoding, force_types)
  File "build/bdist.linux-x86_64/egg/anymarkup_core/__init__.py", line 88, in parse
    raise AnyMarkupError(e)
AnyMarkupError: AnyMarkupError: caught <class 'yaml.scanner.ScannerError'>: mapping values are not allowed here
  in "<file>", line 17, column 13

All good now 👍

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants