Get collaborator role name #3295

jmgate · 2025-05-22T19:28:44Z

This commits adds a duplicate of get_collaborator_permission(), which, instead of returning data["permission"], returns data["role_name"]. The former only returns "admin", "write", "read", or "none"; the latter now also tells us whether the collaborator has the "maintain" or "triage" role.

I'm using PyGitHub to monitor ~1K repositories under my company's GitHub organization, to ensure they stay in compliance with our policies, e.g., keeping track of the roles of outside collaborators, and making sure they stay at "triage" or below. It's possible to achieve what we need to using get_collaborators() with some combination of affiliation and permission, but the addition of this new function means making significantly fewer API calls and speeding up our monitoring CI jobs by a factor of 2-3.

I realize this is a brute-force addition. If you're happy to accept it as-is, great. If we need to clean things up such that there's less duplication across the two functions, or clearer docstring to differentiate the two, etc., just let me know.

EnricoMi · 2025-06-05T14:03:05Z

Ideally, get_collaborator_permission should return the entire data as some GithubObject, so calling site has access to permission and role_name attributes. But I can't think of a way that doesn't break get_collaborator_permission return type. This is the right name for the method returning the object instead of a string.

jmgate · 2025-06-05T14:22:46Z

I'm afraid there's not an ideal solution, given the GitHub API. get_collaborator_permission is hitting the permission endpoint, but that returns a data structure, which then has a permission attribute that this method should return for it to make sense from a user perspective. E.g., you wouldn't want the user to have to do something like

permission = repo.get_collaborator_permission("user")["permission"]
#                                  ^^^^^^^^^^          ^^^^^^^^^^
# Why do I need to ask for the permission twice?

That said, we could create a _get_collaborator_permission_data() method to hit the endpoint, and then do something along the lines of

def get_collaborator_permission(...):
    return _get_collaborator_permission_data(...)["permission"]
def get_collaborator_role_name(...):
    return _get_collaborator_permission_data(...)["role_name"]

jmgate · 2025-06-23T13:12:20Z

@EnricoMi, would you like me to change up this PR such that there's just a get_collaborator_permission_data() method, and then the user can grab whatever details they like from the resulting data structure?

EnricoMi · 2025-07-17T19:32:37Z

You are right, this is difficult to resolve, given Githubs API response. This can be fixed separately, lets keep this simple for your additional use case.

EnricoMi · 2025-07-17T19:36:19Z

Can you please add a unit test, similar to Repository.testCollaboratorPermission.

You can copy existing replay data tests/ReplayData/Repository.testCollaboratorPermission.txt and add the expected role_name attribute to the test data.

This commits adds a duplicate of `get_collaborator_permission()`, which, instead of returning `data["permission"]`, returns `data["role_name"]`. The former only returns "admin", "write", "read", or "none"; the latter now also tells us whether the collaborator has the "maintain" or "triage" role. I'm using PyGitHub to monitor ~1K repositories under my company's GitHub organization, to ensure they stay in compliance with our policies, e.g., keeping track of the roles of outside collaborators, and making sure they stay at "triage" or below. It's possible to achieve what we need to using `get_collaborators()` with some combination of `affiliation` and `permission`, but the addition of this new function means making significantly fewer API calls and speeding up our monitoring CI jobs by a factor of 2-3. I realize this is a brute-force addition. If you're happy to accept it as-is, great. If we need to clean things up such that there's less duplication across the two functions, or clearer docstring to differentiate the two, etc., just let me know.

jmgate · 2025-07-21T14:10:05Z

@EnricoMi, is the test and corresponding replay data in 6e0913f sufficient?

EnricoMi

LGTM!

github-actions · 2025-07-21T17:05:34Z

Test Results

8 files ±0 8 suites ±0 4m 47s ⏱️ ±0s
1 000 tests +1 1 000 ✅ +1 0 💤 ±0 0 ❌ ±0
11 075 runs +8 11 074 ✅ +8 1 💤 ±0 0 ❌ ±0

Results for commit 1be6753. ± Comparison against base commit 03e47ae.

♻️ This comment has been updated with latest results.

github/Repository.py

EnricoMi

LGTM!

jmgate · 2025-07-21T20:45:01Z

Thanks so much for your help getting these changes in, @EnricoMi 😀

This commits adds a duplicate of `get_collaborator_permission()`, which, instead of returning `data["permission"]`, returns `data["role_name"]`. The former only returns "admin", "write", "read", or "none"; the latter now also tells us whether the collaborator has the "maintain" or "triage" role. I'm using PyGitHub to monitor ~1K repositories under my company's GitHub organization, to ensure they stay in compliance with our policies, e.g., keeping track of the roles of outside collaborators, and making sure they stay at "triage" or below. It's possible to achieve what we need to using `get_collaborators()` with some combination of `affiliation` and `permission`, but the addition of this new function means making significantly fewer API calls and speeding up our monitoring CI jobs by a factor of 2-3. I realize this is a brute-force addition. If you're happy to accept it as-is, great. If we need to clean things up such that there's less duplication across the two functions, or clearer docstring to differentiate the two, etc., just let me know. --------- Co-authored-by: Enrico Minack <github@enrico.minack.dev>

jmgate force-pushed the get-collaborator-role-name branch from 4ede6cd to 6e0913f Compare July 21, 2025 14:06

EnricoMi approved these changes Jul 21, 2025

View reviewed changes

EnricoMi enabled auto-merge July 21, 2025 17:02

EnricoMi requested changes Jul 21, 2025

View reviewed changes

github/Repository.py Outdated Show resolved Hide resolved

EnricoMi added 2 commits July 21, 2025 20:39

Fix type issue, simpify isinstance

ab60027

Merge branch 'main' into get-collaborator-role-name

1be6753

EnricoMi approved these changes Jul 21, 2025

View reviewed changes

EnricoMi added this pull request to the merge queue Jul 21, 2025

Merged via the queue into PyGithub:main with commit 2d4785d Jul 21, 2025
22 checks passed

EnricoMi added the improvement label Jul 30, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Get collaborator role name #3295

Get collaborator role name #3295

Uh oh!

jmgate commented May 22, 2025

Uh oh!

EnricoMi commented Jun 5, 2025

Uh oh!

jmgate commented Jun 5, 2025

Uh oh!

jmgate commented Jun 23, 2025

Uh oh!

EnricoMi commented Jul 17, 2025

Uh oh!

EnricoMi commented Jul 17, 2025

Uh oh!

jmgate commented Jul 21, 2025

Uh oh!

EnricoMi left a comment

Uh oh!

github-actions bot commented Jul 21, 2025 •

edited

Loading

Uh oh!

Uh oh!

EnricoMi left a comment

Uh oh!

Uh oh!

jmgate commented Jul 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Get collaborator role name #3295

Get collaborator role name #3295

Uh oh!

Conversation

jmgate commented May 22, 2025

Uh oh!

EnricoMi commented Jun 5, 2025

Uh oh!

jmgate commented Jun 5, 2025

Uh oh!

jmgate commented Jun 23, 2025

Uh oh!

EnricoMi commented Jul 17, 2025

Uh oh!

EnricoMi commented Jul 17, 2025

Uh oh!

jmgate commented Jul 21, 2025

Uh oh!

EnricoMi left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Uh oh!

EnricoMi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jmgate commented Jul 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Jul 21, 2025 •

edited

Loading