Update dist-git domain names mapping to support Forgejo dist-git - #1010
Update dist-git domain names mapping to support Forgejo dist-git#1010betulependule wants to merge 5 commits into
Conversation
Dist-git URLs should remain the same once the migration from Pagure to Fedora Forge takes place. These links are now included in the service mapping of `ForgejoService`. This step is necessarry for `Config.load_authentication()` in packit to work when authenticating against dist-git. These hostname mappings have been removed from `PagureService`. The `get_service_class_or_none` method doesn't expect for two service classes to map to the same hostname, so having the same mapping to the same hostname would yield unexpected behavior (only the class corresponding to the given hostname would be returned).
"https://src.fedoraproject.org" will no longer be associated with Pagure after the migration. The value of `instance_url` has been changed to "https://pagure.io".
The two hostnames will no longer be associated with Pagure after the migration of dist-git and should be removed. The edit to the test recording is more of a hot fix. It would be probably better to add a new recording, but not sure if it's worth it.
|
🤖 Finished Review · ✅ Success · Started 9:03 AM UTC · Completed 9:22 AM UTC Commit: |
|
Build succeeded. ✔️ pre-commit SUCCESS in 3m 29s |
ReviewFindingsMedium
Low
Labels: PR modifies Forgejo and Pagure service mappings for dist-git domain migration |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the review comment for full details.
Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:
ogr/services/pagure/service.py:28: [medium] breaking-change-service-mapping
The @use_for_service decorators for src.fedoraproject.org, src.stg.fedoraproject.org, pkgs.fedoraproject.org, and pkgs.stg.fedoraproject.org have been moved from PagureService to ForgejoService. Downstream consumers calling get_service_class() or get_project() with these URLs will receive ForgejoService/ForgejoProject instead of PagureService/PagureProject. This is the intended change per issue #997 but is a backward-incompatible API surface change that should be clearly documented in the release.
Suggested fix: Ensure the release notes clearly document this mapping change and coordinate with downstream consumers (packit-service, etc.).
ogr/services/pagure/service.py(file-level): Line 39 · [medium] breaking-change-default-parameter
PagureService.init default instance_url changed from 'https://src.fedoraproject.org' to 'https://pagure.io'. Code constructing PagureService() without an explicit instance_url will now connect to pagure.io instead of src.fedoraproject.org.
Suggested fix: Document this default change in release notes. Verify downstream consumers that create PagureService without explicit instance_url.
ogr/services/pagure/project.py(file-level): Line 379 · [medium] breaking-change-is-private
PagureProject.is_private() removed src.fedoraproject.org and src.stg.fedoraproject.org from its hardcoded allowlist. If anyone explicitly creates PagureService(instance_url='https://src.fedoraproject.org') and calls is_private(), it will raise OperationNotSupported instead of returning False.
Suggested fix: Note this behavioral change in release notes.
fullsend-ai-review
Bot
added
requires-manual-review
2 participants
Fixes #997
Merge after the migration of dist-git.
RELEASE NOTES BEGIN
ogrnow maps the dist-git domain names to the Forgejo classes instead of Pagure. Accordingly, the default value ofPagureService.instance_urlhas changed from the dist-git domain topagure.io, andPagureProject.is_private()no longer considers the dist-git domains.RELEASE NOTES END