Pass keyword arguments to TDSCatalog object#303
Pass keyword arguments to TDSCatalog object#303kahemker wants to merge 1 commit intoUnidata:mainfrom
Conversation
…ssed into session_manager.
dopplershift
left a comment
dopplershift
left a comment
There was a problem hiding this comment.
Thanks for the contribution! And apologies on the delay in responding. Just a couple minor things here and this can go in.
|
|
||
| """ | ||
| self.session = session_manager.create_session() | ||
| session_manager.set_session_options(**kwargs) |
There was a problem hiding this comment.
I don't think this line is doing any good since it sets options after self.session is created. I'm pretty sure it's only the get line below that's doing any good.
There was a problem hiding this comment.
This line is needed for my proxy setup. I use this line to modify the HTTP headers on the request session. If you look inside http_util.py, the set_session_options function adds the keywords I need. Specifically the "User-Agent" parameter in the header.
There was a problem hiding this comment.
If that's the case, you really should call set_session_options() before calling create_session(). set_session_options only affects subsequent calls to create_session.
There was a problem hiding this comment.
Interesting... if I comment out set_session_options after create_session() my code fails from the proxy blocking the request. I expect this.
If I move the set_session_options line before create_session() my code fails from the proxy blocking the request on Line 282 self.session.get(catalog_url, **kwargs). The same failure if comment out set_session_options. This is unexpected based on what you told me.
I'm not going to pretend like I understand what is going on. I just know the code in this PR is the only thing that works for my network.
There was a problem hiding this comment.
Can you show the relevant sample of code you're using with siphon? At least the code that creates a TDSCatalog up to the point that fails? I feel like there's something I'm missing here... (You can add that in a stand-alone comment below since I think we're getting a little long-winded here)
There was a problem hiding this comment.
Just added another comment with some relevant code samples. Sorry it took so long, ran into some issues with cftime package updating and manifesting another bug in pvlib.
|
I made another repo that shows how I am using this PR. I am doing solar forecasting with PVLIB which uses Siphon to gather data. I had to modify PVLIB to pass additional arguments into Siphon in order to successfully get the request through my corporate proxy. I hope you can follow my readme when creating the test environment. If you don't install |
|
Kyle H seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
3 participants
Description Of Changes
My corporate network does not allow for standard SSL connections to weather forecast servers. I found that by allowing for passing keywords into the TDSCatalog object and then further into the session manager, I could send the appropriate headers and option flags into urllib that allow the get request to succeed.
Checklist