Add galaxy to user agent #17578
Add galaxy to user agent #17578
Conversation
|
This looks like a good start, do you need any help ? |
|
Hi Marius, I've reopened this PR and updated the description. Regarding help, thanks! The |
lib/galaxy/util/user_agent.py
Outdated
| urllib.request.build_opener = new_build_opener | ||
|
|
||
|
|
||
| __append_word_to_user_agent("galaxy") |
There was a problem hiding this comment.
I think we could also include the galaxy version here as well
There was a problem hiding this comment.
I think we could also include the galaxy version here as well
Good idea, I added the galaxy version to the User-Agent.
Now, the primary outstanding TODO is to figure out where to import the user_agent package to get full coverage. I probably should defer to y'all on that one, there's probably a best place that those with deeper knowledge of the codebase would know.
There was a problem hiding this comment.
Hello @hexylena, I'm checking in on this PR. I think it's mostly done, except that someone with deep knowledge of the codebase (probably a regular contributor such as yourself) needs to determine where to import the user_agent package so that the user agent is properly set at all entry points. Please let me know if there's any way I can help.
| def new_default_user_agent(*args): | ||
| return f"{old_default_user_agent(*args)} {word}" | ||
|
|
||
| requests.utils.default_user_agent = new_default_user_agent |
There was a problem hiding this comment.
We'll have to find another way to do this, we shouldn't do monkey patching on a third party library
| opener.addheaders = [modify_user_agent_header(header) for header in opener.addheaders] | ||
| return opener | ||
|
|
||
| urllib.request.build_opener = new_build_opener |
There was a problem hiding this comment.
same as the comment above, w.r.t. the standard library.
| urllib.request.build_opener = new_build_opener | ||
|
|
||
|
|
||
| __append_word_to_user_agent(f"galaxy/{VERSION}") |
There was a problem hiding this comment.
This will be triggered by just importing the module, which may be a surprising side effect in many contexts. It may be better to have a function that can be imported and then called explicitly. That would also simplify testing of this module.
|
We'll likely want to do something like c0a91c3 instead. I'll tweak this a little, but (almost) all modern code should use requests at this code, and that should cover the interaction with dockstore as well. |
Hi, my name is Steve, and I'm an engineer on the Dockstore team. This PR attempts to append the string "galaxy" to the
User-Agentheader of all Galaxy HTTP requests. For example, it modifies the "requests" package'sUser-Agentso it looks like:Galaxy uses both the "requests" and the "urllib" packages, so it also changes urllib's
User-Agentsimilarly.This PR would allow us to differentiate Galaxy requests from other Python-based Dockstore traffic . After this change, we'd be able to better track Galaxy usage and handle its requests more appropriately. For example, it'd be more feasible for us to filter a DOS attack and not affect Galaxy traffic, or to tweak our webservice with a Galaxy-specific accommodation.
In addition to adding the word "galaxy", we could include the Galaxy version, or the host name so that different installations could be more easily differentiated. Your call!
The
User-Agentis modified via the newuser_agentpackage, which patches the appropriate methods in both the "requests" and "urllib" packages when it is imported. For it to work 100% correctly, it must be imported before any HTTP requests are made. I figured it was easiest to import at all of the entry points to the code. The main app and the data_fetch script are covered, but there may be some I've missed...If you want me to try to cook up an automated test, please let me know.
How to test the changes?
(Select all options that apply)
User-Agentheader and you have access to. Check the logs and confirm that theUser-Agentincludes "galaxy".License