backend: skip projects with comps enabled when migrating to pulp #4087
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds functionality to skip projects with comps.xml enabled during the migration to Pulp, which is a necessary step as Pulp does not support this feature. The changes involve adding a new has_comps helper function and integrating it into the main migration script.
My review focuses on improving the efficiency and robustness of the new code. I've identified a performance bottleneck due to repeated API client instantiation within a loop, and a potential issue with overly broad exception handling. Addressing these points will make the migration script more performant and reliable, especially when dealing with a large number of projects.
Pull Request validationFailed🔴 Review - Missing review from a member (2 required) Success🟢 CI - All checks have passed |
nikromen
force-pushed
the
pulp-migration-skip-comps
branch
from
b34fbd3 to
ad3b848
Compare
| return True | ||
| except CoprRequestException as ex: | ||
| log.warning("Failed to query chroot %s: %s", chroot, str(ex)) | ||
| continue |
There was a problem hiding this comment.
I think there should be either raise() or sys.exit(1) here.
There was a problem hiding this comment.
I rather removed the try-except... if it raises an exception, the error message will be outputted anyway
this should in theory work, however I haven't tested it yet, so no mergy please (but ready for review)works:Fix #4027