From 1bfd41415a35d65cf0df5f5db00f740306658377 Mon Sep 17 00:00:00 2001 From: beckermr Date: Wed, 27 Nov 2024 05:44:16 -0600 Subject: [PATCH 1/5] fix: try two public urls for maintainer exists lint --- conda_smithy/lint_recipe.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/conda_smithy/lint_recipe.py b/conda_smithy/lint_recipe.py index 67dbdb909..f9c3d2376 100644 --- a/conda_smithy/lint_recipe.py +++ b/conda_smithy/lint_recipe.py @@ -417,12 +417,20 @@ def _maintainer_exists(maintainer: str) -> bool: return False return True else: - return ( - requests.get( - f"https://api.github.com/users/{maintainer}" - ).status_code - == 200 - ) + # this API request has no token and so has a restrictive rate limit + # return ( + # requests.get( + # f"https://api.github.com/users/{maintainer}" + # ).status_code + # == 200 + # ) + # so we check two public URLs instead. + # 1. github.com/ - this URL works for all users and all orgs + # 2. https://github.com/orgs/ - this URL only works for + # orgs so we make sure it fails + req_profile = requests.get(f"https://github.com/{maintainer}") + req_org = requests.get(f"https://github.com/orgs/{maintainer}") + return req_profile.status_code == 200 and req_org.status_code != 200 @lru_cache(maxsize=1) From e81912f790477292a1ca368a07dfe53b72dd8bcd Mon Sep 17 00:00:00 2001 From: beckermr Date: Wed, 27 Nov 2024 05:48:09 -0600 Subject: [PATCH 2/5] test: add test for org --- tests/test_lint_recipe.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_lint_recipe.py b/tests/test_lint_recipe.py index 4534fd7da..115c584f3 100644 --- a/tests/test_lint_recipe.py +++ b/tests/test_lint_recipe.py @@ -1871,6 +1871,12 @@ def test_maintainer_exists(self): expected_message = 'Recipe maintainer "isuruf" does not exist' self.assertNotIn(expected_message, lints) + lints, _ = linter.lintify_meta_yaml( + {"extra": {"recipe-maintainers": ["conda-forge"]}}, conda_forge=True + ) + expected_message = 'Recipe maintainer "conda-forge" does not exist' + self.assertIn(expected_message, lints) + def test_maintainer_team_exists(self): lints, _ = linter.lintify_meta_yaml( { From ca9acadd694d0b18aa5d8f34bf0aa1b94fd14d68 Mon Sep 17 00:00:00 2001 From: beckermr Date: Wed, 27 Nov 2024 05:54:31 -0600 Subject: [PATCH 3/5] fix: need to use the teams endpoint --- conda_smithy/lint_recipe.py | 4 ++-- tests/test_lint_recipe.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/conda_smithy/lint_recipe.py b/conda_smithy/lint_recipe.py index f9c3d2376..2668a24c1 100644 --- a/conda_smithy/lint_recipe.py +++ b/conda_smithy/lint_recipe.py @@ -426,10 +426,10 @@ def _maintainer_exists(maintainer: str) -> bool: # ) # so we check two public URLs instead. # 1. github.com/ - this URL works for all users and all orgs - # 2. https://github.com/orgs/ - this URL only works for + # 2. https://github.com/orgs//teams - this URL only works for # orgs so we make sure it fails req_profile = requests.get(f"https://github.com/{maintainer}") - req_org = requests.get(f"https://github.com/orgs/{maintainer}") + req_org = requests.get(f"https://github.com/orgs/{maintainer}/teams") return req_profile.status_code == 200 and req_org.status_code != 200 diff --git a/tests/test_lint_recipe.py b/tests/test_lint_recipe.py index 115c584f3..6cd464ca6 100644 --- a/tests/test_lint_recipe.py +++ b/tests/test_lint_recipe.py @@ -1872,7 +1872,8 @@ def test_maintainer_exists(self): self.assertNotIn(expected_message, lints) lints, _ = linter.lintify_meta_yaml( - {"extra": {"recipe-maintainers": ["conda-forge"]}}, conda_forge=True + {"extra": {"recipe-maintainers": ["conda-forge"]}}, + conda_forge=True, ) expected_message = 'Recipe maintainer "conda-forge" does not exist' self.assertIn(expected_message, lints) From 66e2786d2fa98882a20bae528fffe36a120e5a90 Mon Sep 17 00:00:00 2001 From: beckermr Date: Wed, 27 Nov 2024 06:22:00 -0600 Subject: [PATCH 4/5] fix: ignore redirects and use more specific urls --- conda_smithy/lint_recipe.py | 32 ++++++++++++++++++++++++++------ tests/test_lint_recipe.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/conda_smithy/lint_recipe.py b/conda_smithy/lint_recipe.py index 2668a24c1..8938fdb1c 100644 --- a/conda_smithy/lint_recipe.py +++ b/conda_smithy/lint_recipe.py @@ -413,9 +413,18 @@ def _maintainer_exists(maintainer: str) -> bool: gh = _cached_gh() try: gh.get_user(maintainer) + is_user = True except github.UnknownObjectException: - return False - return True + is_user = False + + # for w/e reason, the user endpoint returns an entry for orgs + # however the org endpoint does not return an entry for users + # so we have to check both + try: + gh.get_organization(maintainer) + is_org = True + except github.UnknownObjectException: + is_org = False else: # this API request has no token and so has a restrictive rate limit # return ( @@ -425,12 +434,23 @@ def _maintainer_exists(maintainer: str) -> bool: # == 200 # ) # so we check two public URLs instead. - # 1. github.com/ - this URL works for all users and all orgs + # 1. github.com/?tab=repositories - this URL works for all users and all orgs # 2. https://github.com/orgs//teams - this URL only works for # orgs so we make sure it fails - req_profile = requests.get(f"https://github.com/{maintainer}") - req_org = requests.get(f"https://github.com/orgs/{maintainer}/teams") - return req_profile.status_code == 200 and req_org.status_code != 200 + # we do not allow redirects to ensure we get the correct status code + # for the specific URL we requested + req_profile = requests.get( + f"https://github.com/{maintainer}?tab=repositories", + allow_redirects=False, + ) + is_user = req_profile.status_code == 200 + req_org = requests.get( + f"https://github.com/orgs/{maintainer}/teams", + allow_redirects=False, + ) + is_org = req_org.status_code == 200 + + return is_user and not is_org @lru_cache(maxsize=1) diff --git a/tests/test_lint_recipe.py b/tests/test_lint_recipe.py index 6cd464ca6..b49f19b31 100644 --- a/tests/test_lint_recipe.py +++ b/tests/test_lint_recipe.py @@ -1878,6 +1878,35 @@ def test_maintainer_exists(self): expected_message = 'Recipe maintainer "conda-forge" does not exist' self.assertIn(expected_message, lints) + def test_maintainer_exists_no_token(self): + gh_token = os.environ.get("GH_TOKEN", None) + try: + if "GH_TOKEN" in os.environ: + del os.environ["GH_TOKEN"] + + lints, _ = linter.lintify_meta_yaml( + {"extra": {"recipe-maintainers": ["support"]}}, + conda_forge=True, + ) + expected_message = 'Recipe maintainer "support" does not exist' + self.assertIn(expected_message, lints) + + lints, _ = linter.lintify_meta_yaml( + {"extra": {"recipe-maintainers": ["isuruf"]}}, conda_forge=True + ) + expected_message = 'Recipe maintainer "isuruf" does not exist' + self.assertNotIn(expected_message, lints) + + lints, _ = linter.lintify_meta_yaml( + {"extra": {"recipe-maintainers": ["conda-forge"]}}, + conda_forge=True, + ) + expected_message = 'Recipe maintainer "conda-forge" does not exist' + self.assertIn(expected_message, lints) + finally: + if gh_token is not None: + os.environ["GH_TOKEN"] = gh_token + def test_maintainer_team_exists(self): lints, _ = linter.lintify_meta_yaml( { From c8a23e5a0f749204dda5eb70e011991b894e7419 Mon Sep 17 00:00:00 2001 From: beckermr Date: Wed, 27 Nov 2024 06:31:38 -0600 Subject: [PATCH 5/5] doc: add news item --- news/2171-fix-maintainer_exists.rst | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 news/2171-fix-maintainer_exists.rst diff --git a/news/2171-fix-maintainer_exists.rst b/news/2171-fix-maintainer_exists.rst new file mode 100644 index 000000000..f44239008 --- /dev/null +++ b/news/2171-fix-maintainer_exists.rst @@ -0,0 +1,24 @@ +**Added:** + +* + +**Changed:** + +* + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* Fixed a bug in checking in recipe maintainers exist without a GitHub token + where the anonymous API would run out of requests. (#2171) + +**Security:** + +*