Skip to content

Conversation

@bhearsum
Copy link
Contributor

@bhearsum bhearsum commented Nov 27, 2025

Due to the issue described in #729, this PR seeks to drop the aiomemoizettl dependency. It does this by using a documented API that supports tokens to avoid the need to memoize (because we have no fears over rate limiting). It's unclear if this API existed or not when we first added that dependency.

After writing this, I realized that we may want to memoize anyways, to avoid unnecessary burning requests with our token, so we could consider doing so some other way rather than (or in addition to) switching APIs.

@bhearsum bhearsum force-pushed the push-qszwnoysmsro branch 2 times, most recently from de59798 to 75476e2 Compare November 27, 2025 21:26
@bhearsum
Copy link
Contributor Author

There's at least one problem with this, which is that the Github API is inconsistent with what it returns for the pulls endpoint. The documentation claims that: "If the commit is not present in the default branch, it will return merged and open pull requests associated with the commit.", but I've found this not to be the case. For example, https://api.github.com/repos/mozilla-releng/scriptworker/commits/3eefdc8492f6502c3354067812880479157fdcb6/pulls returns an empty json list. It may have something to do with the pull request being opened from a fork? In any case, we'll need to make sure to handle that, as well as the case where an unmerged pull request is returned in the response.

There may be other issues, and I also haven't tested this in any real world scenario yet.

@Eijebong
Copy link
Contributor

Eijebong commented Dec 1, 2025

I think it's because despite the commit targetting this repo, it's actually in Julien's repo. https://api.github.com/repos/jcristau/scriptworker/commits/0123e3ddb66e0032dccf807a76915600e9f509ae/pulls vs https://api.github.com/repos/mozilla-releng/scriptworker/commits/0123e3ddb66e0032dccf807a76915600e9f509ae/pulls (first one works, second one doesn't)

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.

2 participants