Pull Request contains a script for building OpenShift template based on Dockerfile.
Feel free to review it.
It looks fine after a brief look. What I'm really missing is documentation how to use such script. I tried it and got this:
$ ./build_oc_template.py --dockerfile ~/.../Dockerfile ghost Traceback (most recent call last): File "./build_oc_template.py", line 197, in <module> sys.exit(main()) File "./build_oc_template.py", line 193, in main otg.run() File "./build_oc_template.py", line 177, in run self._load_oc_template(docker_dict) File "./build_oc_template.py", line 108, in _load_oc_template with open(self.oc_template, 'r') as f: TypeError: coercing to Unicode: need string or buffer, NoneType found
I figured that I'm missing the template (why is it not included? or better said, why user is not informed how to obtain it). So I downloaded the one which we have in container-image-template repo. And got this:
$ ./build_oc_template.py --dockerfile ~/g/.../Dockerfile ghost Traceback (most recent call last): File "./build_oc_template.py", line 197, in <module> sys.exit(main()) File "./build_oc_template.py", line 193, in main otg.run() File "./build_oc_template.py", line 177, in run self._load_oc_template(docker_dict) File "./build_oc_template.py", line 114, in _load_oc_template labels = templ['metadata']['annotation'] KeyError: 'annotation'
Could you please write some tests?
New commit is available here: https://pagure.io/modularity/modularity-tools/c/34679514bcc4da43b5be6a52a1f74df9e365e2ac
It seems to be bug in pagure.io.
Will check tomorrow. Just FYI, we definitely don't want pycs in our repository:
pyc
tests/__pycache__/test_oc_template.cpython-27-PYTEST.pyc
modularity/oc_template.pyc
Indeed, pagure refuses to get new commits from the branch. Had to check locally.
Good job with tests, they are all passing: 8 passed in 0.19 seconds
8 passed in 0.19 seconds
I still miss documentation: what is this tool for? How do I use it? How do I run tests? What is the input? What is the output?
I tried to run it, but I was never successful:
./build_oc_template.py --dockerfile ~/packaging/fedora/cockpit/Dockerfile registry.fedoraproject.org/cockpit ('\n%s', IndexError('list index out of range',))
Other image resulted into:
('\n%s', TypeError('coercing to Unicode: need string or buffer, NoneType found',))
It seems that the tool expects a lot of values and doesn't handle well when they are missing. So my advice here is simple: do NOT expect any value and fail with a proper error message:
value X is missing from file Y, cannot continue
2 new commits added
1 new commit added
@ttomecek I have done several updates. Add more tests, like downloading cockpit docker file and testing it. Also LABELS used for atomic are not count as LABEL. I ignore LABEL INSTALL, LABEL UNINSTALL, LABEL RUN. Also I have add more docs and text. Including function documentation.
I would be glad for other test.
Unfortunately I don't have time to review this, can anyone else take a look? @nphilipp @dhodovsk @karsten? I think that @phracek would appreciate comments from other person than me :D
No one objects, let's merge. We can still fix bugs later.
Pull-Request has been merged by ttomecek