Skip to content
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

Fix performance issue #1

Closed
wants to merge 1 commit into from
Closed

Fix performance issue #1

wants to merge 1 commit into from

Conversation

Zai-Kun
Copy link

@Zai-Kun Zai-Kun commented Oct 27, 2023

The gifs.access() function used to retrieve JSON data from the server every time it was called. Now, it will store the JSON data and return it if it has already been retrieved (you can force an update of the data by passing update_list and setting it to true).

@MarcoSa-2000 MarcoSa-2000 self-requested a review December 2, 2023 18:25
@MarcoSa-2000
Copy link
Owner

Hi! Thank you for this, but I want to commit to a model where the gifs file can be easily modified and the modified file is accessed instantly by the application that uses it (for swiftly malicious gifs removal and etc.).

Also, there shouldn't be any performance issues on how the gif is retrieved, the only time where a performance issue is visible is only at the first call to the server; in the following calls it just accesses directly to the url that the server gave, and I'm pretty sure is as fast or faster to any comparable api call (although this is not). Maybe I can make just give the gif file directly without any auth to make it even faster, although on my end doesn't seem really a problem.

@Zai-Kun
Copy link
Author

Zai-Kun commented Dec 2, 2023

Your current code makes at least two requests even after the initial call to the get_gif function. It's sending the requests to retrieve the same JSON file both times, making it inefficient. if we make it so it stores the data it retrieves from the first request, it would only send one request to the server every time get_gif function is called, hence making it 2x times faster. Here is the code that implements this:

def get_gif(self, category: str) -> str:
    if type(category) is not str: # making sure it's str, and not any other type
        raise errors.CategoryIntegral(category)
    gifs_data = gifs.access() # calling the api only once
    if category.lower() in list(gifs_data.keys()) or category.lower() == 'random':
        if category.lower() == 'random':
            gif_list = []
            for key, gif_url in gifs_data.items():
                for urls in gif_url:
                    gif_list.append(urls[0])
            gif = gif_list
        else:
            gifs_list = gifs_data[category.lower()]
            gif = [gif_url[0] for gif_url in gifs_list]
        gif = random.choice(gif)
    elif category.lower() not in list(gifs_data.keys()):
        raise errors.CategoryError(category)
    else:
        raise errors.CategoryUnknown(category)
    return gif

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