#2 Script for building OpenShift template
Merged by ttomecek. Opened by phracek.
https://pagure.io/modularity/modularity-tools/ master

Download 2.patch
no initial comment

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:

  • 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

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

  • Add gitignore and more tests for build_oc_template
  • Created module under modularity package

1 new commit added

  • Several fixes.

@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

Metadata