-
Notifications
You must be signed in to change notification settings - Fork 13
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
Refactor Copr support into modules #344
Conversation
Will require #345 to go in first and be rebased on how the modules work there. |
9681962
to
8b9bf8d
Compare
d49c59f
to
cc94cfe
Compare
I have updated this with a rebase onto master, and pushing up all the changes from testing builds locally. |
I don't understand why this only fails on 3.11. The failing line: Where I have the mock printing out JSON like the |
% ./tests/fixtures/mockbin/copr-cli
/home/evgeni/Devel/theforeman/obal/./tests/fixtures/mockbin/copr-cli:10: DeprecationWarning: 'pipes' is deprecated and slated for removal in Python 3.13
from pipes import quote
{} that ain't no valid json? :-) Fix: #349 |
Thanks! I need to squint harder next time. |
This is ready for review. It is helpful to review it with respect to theforeman/foreman-packaging#9290 as that gives it some real world feel. |
2f82d21
to
887c3eb
Compare
@evgeni What would you recommend we do here? Drop 2.7 support or test it via a different method?
|
c80c28f
to
1931a35
Compare
try: | ||
package_info = json.loads(copr_cli(command, config_file=config_file)) | ||
except CoprCliCommandError as error: | ||
if "Error: No package with name {} in copr {}".format(package, project): |
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.
this is if <a string>:
which is always true
, did you forget to match against some output?
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.
Rather than matching here, can we catch this in copr_cli
and implement a specific CoprPackageRetrievalError
exception (which subclasses CoprCliCommandError
?
I'd prefer fixing CI (I started in #352), but that shouldn't be a blocker for now. |
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 posted a few questions inline, but only the if <a string>:
I'd consider blocking. Feel free to merge in my absence once that one is cleaned out.
4131bcf
to
d5de731
Compare
The Python 2.7 tests are currently known to be broken, attempting to be fixed over here -- #352 |
Start of re-factoring into modules based on our learning from Koji interaction. One aspect of this is how we define the data, while I am going to work on tests, I thinking of a data structure like this: