Skip to content

Add branch prefix [RHELDST-36928]#139

Merged
rbikar merged 1 commit intorelease-engineering:masterfrom
necipkavrukoglu:add_branch_prefix
Mar 2, 2026
Merged

Add branch prefix [RHELDST-36928]#139
rbikar merged 1 commit intorelease-engineering:masterfrom
necipkavrukoglu:add_branch_prefix

Conversation

@necipkavrukoglu
Copy link
Contributor

@necipkavrukoglu necipkavrukoglu commented Feb 18, 2026

Currently it's easy to break UBI tooling when branch is created with non-standard naming.
In order to avoid this, branch_prefix configuration is added as optional parameter.
it is configured in CDN Definitions.
If prefixes do not match, pre load function skips these branches, load function fails.

@necipkavrukoglu necipkavrukoglu changed the title Add branch prefix [RHELDST-36928] Add branch prefix Feb 18, 2026
Comment on lines +90 to +92
raise ValueError(
f"Branch prefix {prefix} is not matching with cdn-definition prefix {self._branch_prefix}."
)
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't had a chance to review the 3 PRs in detail with full context, but I'm just jumping in to point out this error raising might be problematic (before I forget), given the previous filtering done in _pre_load. We wanted to avoid crashing the ubi sync jobs and warn + skip in the first place (see https://issues.redhat.com/browse/RHELDST-29955 and https://issues.redhat.com/browse/RHELDST-29966, and related PR: https://github.com/release-engineering/ubi-config/pull/82/changes).

Just bringing this to @rbikar's attention, we have chatted a bit about it, but I want to make sure I'm not missing the bigger picture here

Copy link
Member

@rbikar rbikar Feb 19, 2026

Choose a reason for hiding this comment

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

@Gdetrane

thanks for pointing that out.

@necipkavrukoglu
In real code we use typically load_all() - where the required filtering is already done. So this will become a dead code.

And also if one calls only load(), it should also filtered correctly because of pre_load() called during instantiation, so most likely this exception will never be raised.

Maybe you can simulate it in unit test, if there's such edge case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with both of you @rbikar , @Gdetrane. I implemented since I saw regex match for branch in upper lines.
Removed it.

@necipkavrukoglu necipkavrukoglu changed the title Add branch prefix Add branch prefix [RHELDST-36928] Feb 19, 2026
@codecov
Copy link

codecov bot commented Feb 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (0e20210) to head (454927c).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #139   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           13        13           
  Lines          365       371    +6     
=========================================
+ Hits           365       371    +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Gdetrane
Copy link
Contributor

Same thing applies here, just like in release-engineering/ubi-manifest#322, always make sure to run all test environments in tox

@rbikar
Copy link
Member

rbikar commented Feb 24, 2026

@necipkavrukoglu

pylint seems to complain on some added code, can you have a look? https://github.com/release-engineering/ubi-config/actions/runs/22175017818/job/64165115081?pr=139

@necipkavrukoglu
Copy link
Contributor Author

@rbikar,

pylint seems to complain on some added code, can you have a look? https://github.com/release-engineering/ubi-config/actions/runs/22175017818/job/64165115081?pr=139

Right, missed one line linting, fixed

@Gdetrane

Same thing applies here, just like in release-engineering/ubi-manifest#322, always make sure to run all test environments in tox

There is another merge request for ubi-config which needs to be merged first in order to not have failures related with it.

Added comparison of CDN Definition prefix and Git branch name prefix. Skips in pre load and fails in load if they dont not match.
@rbikar rbikar merged commit 0ff8bf7 into release-engineering:master Mar 2, 2026
8 checks passed
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.

3 participants