-
-
Notifications
You must be signed in to change notification settings - Fork 688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simplify PEP cog to use PEP API #2166
Conversation
I'd vote for removing it yea. We could add it to bot-core for future use if needed too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonderful work, wookie. I've reviewed and tested, I just have one small question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🌟
There's no rush to get this merged so we should wait for python/peps#2584 to be resolved. |
Yeah, I'd suggest waiting if you can, since until #2584 is resolved (hopefully soon...™) the "API" is experimental and undocumented, and the path/endpoint, structure, etc. may change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks wookie :D
One tiny comment for you :p
self.peps: Dict[int, str] = {} | ||
# To avoid situations where we don't have last datetime, set this to now. | ||
self.last_refreshed_peps: datetime = datetime.now() | ||
self.peps: dict[int, dict[str, Optional[str]]] = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be a good place to start using PEP 604 :D
I'm going to close this PR for now as it's been stalled for a while, hopefully it can be reused at some point in the future. |
There is a new PEP API (https://peps.python.org/api/peps.json) that saves us having to manually scrape files from github, largely simplifying the implementation.
As far as I can tell this was also the only usage of https://github.com/python-discord/bot/blob/5fe110b9a49144480ebca34dec65de91753994ec/bot/utils/caching.py, do we want to remove that given it's no longer used?