Skip to content

Conversation

brad90four
Copy link
Member

@brad90four brad90four commented Sep 7, 2021

Relevant Issues

PR
Closes #605
Issue
Closes #602

Description

Take the user's input and attempt to fuzzy match the query against the WTF Python's repository headers as defined in the README.md for the repository. If a match is found, the command will send an embed containing a link to the WTF Python's repository header.

If no match is found above the minimum certainty threshold, an error message is sent in an embed that states the user's query could not be found.

Reasoning

Approved by Staff / closes open issue / continues previous PR.

Screenshots from original PR

image

Did you:

@Shom770
Copy link
Contributor

Shom770 commented Sep 7, 2021

image
@brad90four Would you know as to why this is happening?

@brad90four brad90four marked this pull request as draft September 7, 2021 14:52
@brad90four
Copy link
Member Author

image
@brad90four Would you know as to why this is happening?

Nope, I went ahead and marked as draft if it is not working. I will need to dive deeper into debugging, I am currently away from my testing setup.

@Shom770
Copy link
Contributor

Shom770 commented Sep 7, 2021

image
@brad90four Would you know as to why this is happening?

Nope, I went ahead and marked as draft if it is not working. I will need to dive deeper into debugging, I am currently away from my testing setup.

I found the issue -- the minimum certainty is too high.

Right now it's at 75, and when you do .wtf del, it finds the string, but the certainty is only 60%

@Xithrius Xithrius changed the title Add WTF Python Command - Continuation of #605 Add WTF Python Command Sep 7, 2021
@Xithrius Xithrius self-requested a review September 7, 2021 20:42
Comment on lines 53 to 92
@tasks.loop(hours=1)
async def fetch_readme(self) -> None:
"""Gets the content of README.md from the WTF Python Repository."""
failed_tries = 0

for x in range(FETCH_TRIES):
async with self.bot.http_session.get(f"{WTF_PYTHON_RAW_URL}README.md") as resp:
log.trace("Fetching the latest WTF Python README.md")

if resp.status == 200:
raw = await resp.text()
self.parse_readme(raw)
log.debug(
"Successfully fetched the latest WTF Python README.md, "
"breaking out of retry loop"
)
break

else:
failed_tries += 1
log.debug(
"Failed to get latest WTF Python README.md on try "
f"{x}/{FETCH_TRIES}. Status code {resp.status}"
)

if failed_tries == 3:
log.error("Couldn't fetch WTF Python README.md after 3 tries, unloading extension.")
action = Action.UNLOAD
else:
action = Action.LOAD

verb = action.name.lower()
ext = "bot.exts.utilities.wtf_python"

try:
action.value(self.bot, ext)
except (commands.ExtensionAlreadyLoaded, commands.ExtensionNotLoaded):
log.debug(f"Extension `{ext}` is already {verb}ed.")
else:
log.debug(f"Extension {verb}ed: `{ext}`.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think any discussion spawned from the last one-

You should add an attribute to the cog: last_fetched which will be a datetime of when this was last updated.
Then, when you use the command and self.last_fetched + datetime.timedelta(hour=1) < datetime.datetime.now() you can use asyncio.create_task(self.fetch_readme) (don't await it) to start a task fetching it in the background.

We still want to keep the response time good, so by using a task we don't need to wait for its completion which means we can return what's cached so far.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'm getting stuck on this one. I haven't used asyncio yet so maybe that is what is tripping me up.
Also, how can I get the "last" time the cog was loaded instead of just calling it immediately?

Here is what I have so far:

class WTFPython(commands.Cog):
    """Cog that allows getting WTF Python entries from the WTF Python repository."""

    def __init__(self, bot: Bot):
        self.bot = bot
        self.headers: dict[str, str] = dict()
        self.last_fetched = datetime.datetime.now()  # this is just the current time, how to access last load?
        log.debug(f"{self.last_fetched = }")

        if self.last_fetched + datetime.timedelta(seconds=10) < datetime.datetime.now():  # this will never be true currently
            log.debug("Last loading time was within x amount of time.")
            asyncio.create_task(self.fetch_readme)  # is this correct?

    # cut out most everything from the function, this is what is left
    async def fetch_readme(self) -> None:
        """Gets the content of README.md from the WTF Python Repository."""
        async with self.bot.http_session.get(f"{WTF_PYTHON_RAW_URL}README.md") as resp:
            log.trace("Fetching the latest WTF Python README.md")
            if resp.status == 200:
                raw = await resp.text()
                self.parse_readme(raw)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking something more like this:

README_REFRESH = 60  # Amount of minutes

class WTFPython(commands.Cog):
    """Cog that allows getting WTF Python entries from the WTF Python repository."""

    def __init__(self, bot: Bot):
        self.bot = bot
        self.headers: dict[str, str] = dict()
        self.last_fetched = datetime.datetime.now()
        log.debug(f"{self.last_fetched = }")

        asyncio.create_task(self.fetch_readme)  # Start a first task

    async def fetch_readme(self) -> None:
        """Gets the content of README.md from the WTF Python Repository."""
        refresh = self.last_fetched + datetime.timedelta(minutes=README_REFRESH)

        if refresh > datetime.datetime.now() and self.headers:  # We also want to fetch if we haven't previously
            return  # Our cache should be up-to-date

        async with self.bot.http_session.get(f"{WTF_PYTHON_RAW_URL}README.md") as resp:
            log.trace("Fetching the latest WTF Python README.md")
            if resp.status == 200:
                raw = await resp.text()
                self.parse_readme(raw)

You then call asyncio.create_task(self.fetch_readme) every time someone runs the command.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoa that is awesome! Thanks for the super clear example.

verb = action.name.lower()
ext = "bot.exts.utilities.wtf_python"
if refresh > datetime.datetime.now() and self.headers:
return # cache should be up-to-date
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return # cache should be up-to-date
return # Cache should be up-to-date


def __init__(self, bot: Bot):
self.bot = bot
self.headers: dict[str, str] = dict()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.headers: dict[str, str] = dict()
self.headers: dict[str, str] = {}

Copy link
Member

@Bluenix2 Bluenix2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have the command have some minimum threshold?

I tried it with the start of the Bee Movie script, first few seconds of Nicki Minaj's superbass, first page of the Bible, the first sentence of the FitnessGram PACER Test, a paragraph about the MRSA bacteria, the MIT license and your name....

but it wasn't until I brought out the morse code of "Hello World" and Never Gonna Give You Up music video link that I got a failure.

@brad90four brad90four requested a review from Bluenix2 October 7, 2021 01:38
@Xithrius Xithrius requested a review from Bluenix2 October 7, 2021 19:29
Copy link
Member

@Bluenix2 Bluenix2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 Thanks Brad!

@Xithrius Xithrius enabled auto-merge (squash) October 22, 2021 02:28
@Xithrius Xithrius merged commit 93f8385 into python-discord:main Oct 22, 2021
@Xithrius Xithrius removed the status: needs review Author is waiting for someone to review and approve label Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: backend Related to internal functionality and utilities category: utilities Related to utilities type: feature Relating to the functionality of the application.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Subsection Command for WTFPython
6 participants