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.

Sign up for GitHub

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

Jump to bottom

Allow setting up repo priority #2733

Merged
merged 1 commit into from
Aug 3, 2023
Merged

Allow setting up repo priority #2733

merged 1 commit into from
Aug 3, 2023

Conversation

nikromen
Copy link
Member

@nikromen nikromen commented May 19, 2023 •
edited
Loading

TODO:

  • tests
  • add it to copr-cli
  • some documentation

Fixes #2568

@nikromen nikromen marked this pull request as draft May 19, 2023 15:30
github-advanced-security[bot]
github-advanced-security bot found potential problems May 19, 2023
View reviewed changes
frontend/coprs_frontend/coprs/forms.py Fixed Show fixed Hide fixed
frontend/coprs_frontend/coprs/models.py Fixed Show fixed Hide fixed
frontend/coprs_frontend/coprs/models.py Fixed Show fixed Hide fixed
frontend/coprs_frontend/coprs/models.py Fixed Show fixed Hide fixed
@nikromen nikromen force-pushed the set-repo-priority branch 2 times, most recently from 1708e77 to 0b2bf73 Compare May 25, 2023 12:08
@nikromen nikromen marked this pull request as ready for review May 25, 2023 12:08
@nikromen nikromen force-pushed the set-repo-priority branch 2 times, most recently from efa6f29 to 21f86d5 Compare May 25, 2023 12:38
@praiskup praiskup requested a review from FrostyX May 29, 2023 10:31
FrostyX
FrostyX reviewed May 31, 2023
View reviewed changes
Copy link
Member

@FrostyX FrostyX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for PR @nikromen,

Overall it looks very good. There are some parts that I think can be simplified, so added some comments to the code.

Apart from that, we should probably squash all the commits together before merging.

rpmbuild/main.py Outdated Show resolved Hide resolved
frontend/coprs_frontend/coprs/models.py Outdated Show resolved Hide resolved
build_aux/per-subdir-makefile Outdated Show resolved Hide resolved
backend/copr_backend/background_worker_build.py Outdated Show resolved Hide resolved
frontend/coprs_frontend/alembic/versions/7d9f6f921fa0_add_repo_priority_field.py Outdated Show resolved Hide resolved
@nikromen nikromen force-pushed the set-repo-priority branch 6 times, most recently from 6619eae to 0aeac65 Compare June 5, 2023 11:55
@nikromen nikromen requested a review from FrostyX June 5, 2023 11:55
FrostyX
FrostyX reviewed Jun 11, 2023
View reviewed changes
rpmbuild/main.py Outdated Show resolved Hide resolved
rpmbuild/man/copr-rpmbuild.1.asciidoc Outdated Show resolved Hide resolved
backend/copr_backend/background_worker_build.py Outdated Show resolved Hide resolved
backend/copr_backend/job.py Outdated Show resolved Hide resolved
rpmbuild/copr_rpmbuild/builders/mock.py Outdated Show resolved Hide resolved
@nikromen nikromen force-pushed the set-repo-priority branch 2 times, most recently from 98f94e9 to d0548bb Compare July 3, 2023 13:21
FrostyX
FrostyX reviewed Jul 23, 2023
View reviewed changes
Copy link
Member

@FrostyX FrostyX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR looks very good, just a couple of minor fixes are needed.

frontend/coprs_frontend/coprs/forms.py Outdated Show resolved Hide resolved
frontend/coprs_frontend/coprs/forms.py Outdated Show resolved Hide resolved
rpmbuild/copr_rpmbuild/builders/mock.py Outdated Show resolved Hide resolved
@nikromen nikromen force-pushed the set-repo-priority branch 4 times, most recently from 1fe543c to ed3c73c Compare July 24, 2023 18:51
@nikromen nikromen force-pushed the set-repo-priority branch 3 times, most recently from 200461d to f66a2ae Compare July 25, 2023 14:48
@nikromen nikromen mentioned this pull request Jul 25, 2023
Merged
3 tasks
@nikromen
Copy link
Member Author

/packit build

@nikromen
Copy link
Member Author

I tested it locally in container and API call returns the priority correctly so it should work as expected now

Screenshot_20230725_164022

@FrostyX PTAL

@praiskup
Copy link
Member

I'd probably prefer having this less intrusive, I tested this and I basically see the number 99 everywhere, which will cause a wave of questions (the number 99 is suspicious, and people will have to study dnf.conf manual page that we do not even point them to).

Can this be done so that None is equivalent to 99 and if that happens, no priority= tag is placed into the repo files?

@nikromen nikromen force-pushed the set-repo-priority branch 2 times, most recently from 3e4f3d4 to 688323e Compare July 31, 2023 18:01
praiskup
praiskup approved these changes Aug 1, 2023
View reviewed changes
Copy link
Member

@praiskup praiskup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@nikromen
Copy link
Member Author

nikromen commented Aug 1, 2023

@praiskup @FrostyX added priority to repo file per standup mtg, PTAL

@praiskup
Copy link
Member

praiskup commented Aug 1, 2023

Can you please update copr_cli/build_config.py, too?

@nikromen
frontend, cli, python, rpmbuild: Add priority=X to repo config
3f9ed71 
This allows users to set priority of their repository via frontend
page or copr-cli. This priority value sets the `priority=X` value
in repo confid in /etc/yum.repos.d/_copr:*repo
@nikromen nikromen merged commit dfd210f into fedora-copr:main Aug 3, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FrostyX FrostyX FrostyX approved these changes

@praiskup praiskup praiskup approved these changes

Assignees

@FrostyX FrostyX

Labels
do-manual-merge-please
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow setting priority=X for the copr_base repo
3 participants
@nikromen @praiskup @FrostyX