Skip to content

backend: respect disable createrepo when forking project to another - #4467

Open
nikromen wants to merge 1 commit into
fedora-copr:mainfrom
nikromen:fix-fork-devel
Open

backend: respect disable createrepo when forking project to another#4467
nikromen wants to merge 1 commit into
fedora-copr:mainfrom
nikromen:fix-fork-devel

Conversation

@nikromen

@nikromen nikromen commented Sep 1, 2026

Copy link
Copy Markdown
Member

this is however issue not related only to uploadrpm but to forking in general

not tested on stg yet

Fix #4461

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 7c85f809-b2bd-4d5f-b191-e7e1f8fe46e1

📥 Commits

Reviewing files that changed from the base of the PR and between 84d35d5 and 19983e9.

📒 Files selected for processing (2)
  • backend/copr_backend/storage.py
  • frontend/coprs_frontend/coprs/logic/actions_logic.py

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The fork flow now propagates destination development settings to repository metadata generation and includes destination owner, project, and development values in fork action data.

Changes

Fork configuration propagation

Layer / File(s) Summary
Repository metadata setting propagation
backend/copr_backend/storage.py
BackendStorage.fork_project passes self.devel to call_copr_repo when generating metadata for the forked chroot.
Fork action data propagation
frontend/coprs_frontend/coprs/logic/actions_logic.py
send_fork_copr adds the destination owner name, project name, and development mode to the fork action data.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 19983

Forking now preserves the destination project's repository-generation setting, keeping repository handling consistent with the destination configuration. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: praiskup, frostyx

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the backend change and its purpose: preserving the disabled createrepo setting during project forks.
Description check ✅ Passed The description relates the change to forking behavior, uploadrpm scope, staging status, and issue #4461.
Linked Issues check ✅ Passed The changes pass the destination chroot's devel setting through the fork flow and repository call. This directly supports honoring the destination project's disabled createrepo configuration during fo…
Out of Scope Changes check ✅ Passed The changes are limited to fork-related backend and frontend data propagation. No unrelated code changes are identified.
Full details: Linked Issues check

Explanation

The changes pass the destination chroot's devel setting through the fork flow and repository call. This directly supports honoring the destination project's disabled createrepo configuration during forks [#4461].

  • Fix all pre-merge checks with AI

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nikromen

nikromen commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

/packit test

1 similar comment
@nikromen

nikromen commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

/packit test


if createrepo and not call_copr_repo(new_chroot_path, logger=self.log):
if createrepo and not call_copr_repo(new_chroot_path, devel=self.devel,
logger=self.log):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is only for the BackendStorage. I guess we will need the same fix for the PulpStorage as well?

@nikromen

nikromen commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

leaving this as is before we align with mikolaj on this

"storage": src.storage,
"ownername": dst.owner_name,
"projectname": dst.name,
"devel": dst.devel_mode,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

bump BE/FE protocol nunber?

@nikromen nikromen Sep 3, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

before doing any follow-up action I want to clarify whether we really want to fix #4461 this way? Or we leave this behavior as is?

To me currently it makes sense to respect disable createrepo of destination fork project (the way how this PR points) however we have not come to clear conclusion, rather that this could be interpreted in numerous ways, so if at the end of the day this is not needed I'd prefer not to touch this and close the issue

"copr": dst.name,
"builds_map": builds_map,
"storage": src.storage,
"ownername": dst.owner_name,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Curious why we need dst.owner_name twice?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

owner = self.ext_data.get("ownername") in actions.py on line 94 takes it; however not related directly to this fix... but would be nice to fill that value and not having it none

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.

Repodata is refreshed after project fork despite --disable_createrepo true setting

3 participants