-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace bash scripts with python utility package #26
Replace bash scripts with python utility package #26
Conversation
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
Signed-off-by: Aloys Baillet <[email protected]>
from aswfdocker.cli import aswfdocker | ||
|
||
|
||
class TestBuilder(unittest.TestCase): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like test_builder.py is hard coded to use 2019 as the test case, should that be configurable so it can easily be moved forward / backwards, or even across all supported years if you wanted to be completely thorough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been thinking about this a bit and the added complexity scares me more than having to update some tests in 2 years time...
All looks great to me, none of the comments I made are blockers, so if you want I'm happy to go ahead and give final approval so this can go in and start to get used. Also generally it seems you've put in place some interesting infrastructure for Python code that could deserve to be propagated across ASWF projects. Maybe worth writing up a short summary for the TAC list, and could be incorporated in the ASWF sample project documents. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved
Signed-off-by: Aloys Baillet <[email protected]>
Thanks for the review JF! I think I've addressed most of your comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good.
Signed-off-by: Aloys Baillet <[email protected]>
There was a merge conflict which means I had to push another commit which discarded your approval... again... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good!
My apologies in advance for another big PR...
The growing number of bash scripts in this repo was making me uncomfortable and as I needed yet another script to migrate docker package images from the
aswftesting
to theaswf
organisation I thought I would try python...So I've replaced around 1k lines of untestable bash+hcl configuration files with around 1k lines of statically typed python 3 with around 96% test coverage (there are 500 lines in this PR coming from the
Pipfile.lock
file which "locks" in place current dependencies to avoid random breakages when new releases happen).I have described the new python utilities there: https://github.com/aloysbaillet/aswf-docker/blob/add-migrate-script/python/README.md
The python code is all formatted using
black
, statically typed-tested withmypy
, tested withpytest
and PEP8-tested withprospector
, withpre-commit
support to enforce all of this automatically before commiting code, and added corresponding Azure Pipeline tests. Code coverage is also reported in a badge on that python README file.New utilities:
aswfdocker packages
lists all ci packages.aswfdocker images
lists all ci images.aswfdocker migrate
allows migration of docker packages from one organisation to another.aswfdocker build
builds docker ci packages and ci images.aswfdocker getdockerorg
prints the current dockerhub organisation to use.aswfdocker getdockerpush
prints if the images should be pushed.