Adds support for passing User Agent headers and Proxies through env variables.

[Raahul-Singh]

@Cadair Would this suffice?
Also, I need some help with testing the code as I do not fully understand how aiohttp works.

[Cadair]

Can you also add some tests for this?

[Raahul-Singh]

Also, the tests are failing with the message create_connection() got an unexpected keyword argument 'headers'
But the headers is only being passed to the aiohttp.ClientSession() as per the aiohttp docs for
Custom Request Headers
I need some help with this.

[Raahul-Singh]

Can you also add some tests for this?

I can write a test to check if the User-Agent header is being generated as required or not.
Since I can't access the ClientSession object, is there a way to check if the UA header is being passed successfully?
Also, how should I test the proxy settings?

[Cadair]

Have a look at the other tests, I think it's possible to inspect the request received by the "server" in the tests. Not sure about the proxy though.

[Raahul-Singh]

So, I got the UA, proxy tests working, further, I found out that the HTTP UA has a proper protocol. The first white space-delimited word must be the software product name, with an optional slash and version designator. I changed it as such so that the server can actually extract meaningful information from it.

Also, only the FTP tests are failing with the error message exception=TypeError("create_connection() got an unexpected keyword argument 'headers'")
Do you have any idea about it or do I go further sleuthing?

[Raahul-Singh]

So, I got FTP the tests working!
Turns out aioftp.ClientSession does not support a headers parameter.

[Raahul-Singh]

@wtbarnes could you help with testing the proxy setting here?

[wtbarnes]

I would, but am no longer at the institution where I initially encountered this problem 😅

[Raahul-Singh]

I need advice on refactoring the mocking code. It needs asynctest and I think the code could do away with certain things like CoutoutineMock (or can it?)

[Cadair]

Looking good!

[Cadair]

Think this looks good. Anything else you think needs doing @Raahul-Singh ?

[Raahul-Singh]

Think this looks good. Anything else you think needs doing @Raahul-Singh ?

I think I found a problem.
aiohttp.ClienSession.get requires proxy_auth as a aiohttp.BasicAuth object.
This cannot be passed as an environment variable as it'll give
TypeError: str expected, not BasicAuth
From a very brief search, I couldn't find a way to pass the auth values in the env.
I believe, for this, we may require passing these arguments to the constructor

[Raahul-Singh]

Similarly, proxy_header needs to be of type Optional[LooseHeaders] and not str
Passing a dict would give
TypeError: str expected, not dict

[Raahul-Singh]

We can either pass these as args to the constructor or not support proxy_headers and proxy_auth, just the proxy_url

[Cadair]

People can still provide auth and headers as kwargs to enqueue_file? That might be enough? I am not sure how often those things are used. (I haven't used a proxy in about 12 years lol).

We should also document both these features before we merge this PR.

[Raahul-Singh]

One more caveat is that right now I am assuming that the proxy URL has the environment variable name as PROXY. If it is anything else, it'll not be read. The user may name it anything.
Either this has to go in the docs as a limitation, or I run a regex test, checking the keys of the os.environ dictionary for the string PROXY

[Raahul-Singh]

People can still provide auth and headers as kwargs to enqueue_file? That might be enough? I am not sure how often those things are used. (I haven't used a proxy in about 12 years lol).

We should also document both these features before we merge this PR.

Yep. That'll work. I'll remove these from the code and add the documentation about these features.

[Cadair]

Is there a "standard" name for a proxy env var? this suggests we should be using HTTPS_PROXY and HTTP_PROXY (case insensitive).

[Cadair]

Needs docs and thoughts on env var naming.

[Raahul-Singh]

Should I add tests for explicitly passing proxy_auth ?

[Raahul-Singh]

I can't think of anything more that I can add to this. I think we covered most of the stuff.

[Cadair]

You think it's ready?

[Raahul-Singh]

You think it's ready?

Let's see, we have:

• Checked the UA being passed.
• Checked the format of the UA.
• Checked the proxy_url being passed.
• Decided to leave the proxy_auth and proxy_headers to the users.
• Checked if the correct protocol is being used based on the url passed.

I can't think of anything else that may be required.
So, I'd say I'm 95% sure that it's ready. 😅
Maybe a rebase-squash on my part?

[Raahul-Singh]

Do you reckon it's done?

[Cadair]

Yeah I think it looks good :)

The next thing to do is to test out 1.1rc2 with sunpy so we can have people use this with sunpy.