Skip to content

[FIX] use authenticated GitHub API to check maintainers in other branches - #357

Open
yajo wants to merge 1 commit into
OCA:masterfrom
moduon:fix-226-maintainer-other-branches-auth
Open

[FIX] use authenticated GitHub API to check maintainers in other branches#357
yajo wants to merge 1 commit into
OCA:masterfrom
moduon:fix-226-maintainer-other-branches-auth

Conversation

@yajo

@yajo yajo commented Jul 30, 2026

Copy link
Copy Markdown
Member

When a maintainer tried to merge or rebase a migration PR, the bot
looked for maintainers in other branches by downloading each addon
manifest from raw.githubusercontent.com. That endpoint is unauthenticated
and subject to rate limiting, transient errors and caching issues, which
made the check fail intermittently (see #226).

With this change, the authenticated GitHub session already used by the
bot is reused to read manifests via the GitHub Contents API. The previous
raw URL request is kept as a fallback in case the API is unavailable.

Fixes #226

Assisted-by: OpenCode + kimi-k2.7-code

@legalsylvain

Copy link
Copy Markdown
Collaborator

Could you take a look on red CI ?

Thanks !

@legalsylvain
legalsylvain marked this pull request as draft July 30, 2026 17:47
@yajo
yajo force-pushed the fix-226-maintainer-other-branches-auth branch from 060b0c2 to 1480a4c Compare July 31, 2026 11:16
@yajo
yajo marked this pull request as ready for review July 31, 2026 11:17
@yajo

yajo commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

Done, was just formatting.

@yajo

yajo commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

Any chances to review this?

cc @sbidoul

@pedrobaeza

Copy link
Copy Markdown
Member

And this is more important since https://github.com/orgs/community/discussions/206581

@sbidoul

sbidoul commented Sep 8, 2026

Copy link
Copy Markdown
Member

Looks good, but why keep the unauthenticated variant? This adds bloat for no reason that I can see.

@yajo

yajo commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

You mean to set the github token as required?

@sbidoul

sbidoul commented Sep 9, 2026

Copy link
Copy Markdown
Member

Yes. But now that I think of it, I'm not even sure we have ever had a 429 on these requests.

@yajo
yajo force-pushed the fix-226-maintainer-other-branches-auth branch from 1480a4c to 39d489e Compare September 9, 2026 09:06
@yajo

yajo commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

Well, I've changed it and now the token is required.

@yajo
yajo force-pushed the fix-226-maintainer-other-branches-auth branch 2 times, most recently from a908905 to fddf338 Compare September 10, 2026 09:28
…rs in other branches

When a maintainer tried to merge or rebase a migration PR, the bot
looked for maintainers in other branches by downloading each addon
manifest from raw.githubusercontent.com. That endpoint is unauthenticated
and subject to rate limiting, transient errors and caching issues, which
made the check fail intermittently (see OCA#226).

Since the bot already requires a GitHub token for all its operations,
keep only the authenticated GitHub Contents API path and drop the raw URL
fallback. This removes unnecessary code and avoids preserving the very
failure mode this fix addresses.

Fixes OCA#226

Assisted-by: OpenCode + Kimi k2.7-code
@yajo
yajo force-pushed the fix-226-maintainer-other-branches-auth branch from fddf338 to d4640cc Compare September 10, 2026 10:21
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.

Maintainer are not able to merge / rebase their modules when migrating

4 participants