Skip to content
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

Closed
wants to merge 2 commits into from

Conversation

cverna
Copy link
Contributor

@cverna cverna commented Feb 28, 2019

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.

@cverna cverna requested a review from a team as a code owner February 28, 2019 11:21
@cverna
Copy link
Contributor Author

cverna commented Mar 7, 2019

jenkies test


RELEASE: Release name (e.g. F27)

[PACKAGES]: comma separated list of packages to add (e.g. firefox,vim,python)
Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor

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 ☺

Copy link
Contributor Author

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 ?

cverna and others added 2 commits March 14, 2019 10:08
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]>
Copy link
Contributor

@bowlofeggs bowlofeggs left a 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.

@bowlofeggs
Copy link
Contributor

Closing due to inactivity.

@bowlofeggs bowlofeggs closed this Apr 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants