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.

Sign up for GitHub

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

Jump to bottom

Pass keyword arguments to TDSCatalog object #303

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Pass keyword arguments to TDSCatalog object #303

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions siphon/catalog.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ class TDSCatalog(object):

"""

def __init__(self, catalog_url):
def __init__(self, catalog_url, **kwargs):
"""
Initialize the TDSCatalog object.

Expand All @@ -276,9 +276,10 @@ def __init__(self, catalog_url):

kahemker marked this conversation as resolved.
Show resolved Hide resolved
"""
self.session = session_manager.create_session()
session_manager.set_session_options(**kwargs)
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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)

Copy link
Author

Choose a reason for hiding this comment

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

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.


# get catalog.xml file
resp = self.session.get(catalog_url)
resp = self.session.get(catalog_url, **kwargs)
resp.raise_for_status()

# top level server url
Expand Down