-
Notifications
You must be signed in to change notification settings - Fork 197
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
Store critical path packages in bodhi. #3049
Conversation
jenkies test |
|
||
RELEASE: Release name (e.g. F27) | ||
|
||
[PACKAGES]: comma separated list of packages to add (e.g. firefox,vim,python) |
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.
How many critpath packages do you think we have, approximately? I wonder if this will be a burdensome way to add/remove them. I also wonder if we will hit the max length of shell arguments?
The alternative might also be burdensome, however, which would be to have add/remove commands. Unless maybe the add/remove commands still at least support comma separated lists like is done here?
I guess my main concern is whether we will hit the shell limits or not. I don't have a super convenient way to find out which packages are critical path. Bodhi's database has a bool on whether an update is critical path, but that's not a direct way to answer the question. We could probably query PDC to find out, though I think it's data is also not up to date.
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 also recognize that this is how the releng script works.
One option we could consider is giving the bindings the ability to set critpath packages, like this here, and then just use that in the releng script. We could leave this code for other Bodhi deployments to use, if they want. Then we at least know that Fedora won't hit shell limits.
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 of the above to say, I'm fine with doing it this way as long as we have the releng script use the bindings instead of shelling out to this, or as long as we think the list is short enough that we don't have to worry. Either way, I'm happy to merge this ☺
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.
So looking at pdc there are 756 critical path packages for f29 (https://pdc.fedoraproject.org/rest_api/v1/component-branches/?active=true&critical_path=true&name=f29). So yes passing this into the shell might not be super smart :)
The current releng script takes a .txt file as an entry, what do you think about doing the same for the bodhi client ?
bodhi/server/migrations/versions/a96dadf085c2_add_critcal_path_packages.py
Outdated
Show resolved
Hide resolved
This commit add support to associate a list of critical path packages to a release. This commit adds a simple API that allow a use to GET or to set (POST) the list of critical path packages. Co-authored-by: Vismay Golwala <[email protected]> Co-authored-by: Clement Verna <[email protected]> Signed-off-by: Clement Verna <[email protected]>
This commit allow the use of bodhi cli to access the critical path API. This add 2 command critpah list and critpath set. Co-authored-by: Vismay Golwala [email protected] Co-authored-by: Clement Verna [email protected] Signed-off-by: Clement Verna <[email protected]>
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.
There are some pydocstyle and unit test failures.
The pydocstyle failures are these:
bodhi/client/__init__.py:1222 in public function `print_critpath_packages`:
D202: No blank lines allowed after function docstring (found 1)
bodhi/server/migrations/versions/a429f263fc3f_add_critical_path_packages.py:1 at module level:
D400: First line should end with a period (not 's')
bodhi/server/migrations/versions/a429f263fc3f_add_critical_path_packages.py:34 in public function `upgrade`:
D103: Missing docstring in public function
bodhi/server/migrations/versions/a429f263fc3f_add_critical_path_packages.py:56 in public function `downgrade`:
D103: Missing docstring in public function
f28-build : SKIPPED
f28-pydocstyle : FAILED [0:00:24.480490] (exited with code: 1)
The unit test failures seem to be due to fedora-infra/python-fedora#213 not being available for the code to use.
Closing due to inactivity. |
This PR rebases, updates and finishes #2476. Once this is accepted an merged a new PR will be need to drop bodhi's PDC dependency.