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

[Update] Making UI simpler and an OOP refactor #114

Merged
merged 19 commits into from
Nov 21, 2022

Conversation

ayushanand18
Copy link
Collaborator

Overview

This PR is huge and I'm really sorry for that.

Changes introduced

Thanks!

@ayushanand18 ayushanand18 requested a review from 7yl4r November 19, 2022 09:25

Usage::
class NodesResponse:
Copy link
Collaborator

Choose a reason for hiding this comment

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

when instantiating a class without a complex parent, you should inherit from object, i.e.:

NodesResponse(object):

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you so much for the review! I didn't know this fact, and I learned it today. I'll update the PR.

Handles technical details of sending GET request to the API
"""
headers = {
"User-Agent": "Mozilla/5.0 (X11; CrOS x86_64 12871.102.0) AppleWebKit/537.36 (KHTML, like Gecko)\
Copy link
Collaborator

Choose a reason for hiding this comment

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

we might consider setting a user-agent string that specifies pyobis instead of this one. I assume this was copied from a browser?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this is a browser user agent string, but I don't know if there are any UA detection tool into OBIS API. If it would be the case then, any other user agent string might work slower than the browser user strings because that might be used to put rate limits on bot networks.

And, I don't know why but the server response time is too high today for me. This request https://api.obis.org/v3/occurrence?taxonid=2&offset=0&mof=False&size=1 is taking 10.69 s waiting for server response while only 1.63 ms for content download. This is something unusual. I tried clearing DNS Cache, changing IPs, also tried VPNs, but the time taken is same.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe this is something related to OBIS performing internal searches for all records and returning only the specified user size. This could be a critical bottleneck before pyobis being performant, but is a backend issue (something as a high-level design problem). I tested taxonids with lesser records footprint and the server wait time was nearly equal to content download time for a similar number of records. I analysed many requests for different number of records and found that the server wait time was a significant portion from 50% to taking nearly 95% of the total time for higher taxons. Therefore I came to the conclusion that for most users, more than network speed, server response time is a bigger bottleneck.

@ayushanand18
Copy link
Collaborator Author

ayushanand18 commented Nov 21, 2022

Interestingly, I have started playing with Request Headers and I find that some minor tweaks can increase performance by more than 25%.
I tested this response header:

headers = {
        "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko)\
         Chrome/107.0.0.0 Safari/537.36 Edg/107.0.1418.52",
        "Accept-Encoding": "gzip, deflate, br",
        "Host": "api.obis.org",
        "Connection": "keep-alive",
    }

And found that it increased speed by more than 27%.

@7yl4r 7yl4r closed this Nov 21, 2022
@7yl4r 7yl4r reopened this Nov 21, 2022
@7yl4r 7yl4r merged commit 4eb3820 into iobis:main Nov 21, 2022
@ayushanand18 ayushanand18 deleted the oops-refactor branch November 22, 2022 12:03
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