Skip to content

Add option to pass cookies to WMTS #995

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ashergg
Copy link

@ashergg ashergg commented May 14, 2025

Added an option to pass cookies to the WMTS class. This can help when the server requires specific cookies.

Copy link
Contributor

@geographika geographika left a comment

Choose a reason for hiding this comment

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

Thanks @ashergg. A couple of comments to address. Ideall0,y a test would be added too.

owslib/wmts.py Outdated
@@ -136,7 +136,7 @@ def __getitem__(self, name):
raise KeyError("No content named %s" % name)

def __init__(self, url, version='1.0.0', xml=None, username=None, password=None,
parse_remote_metadata=False, vendor_kwargs=None, headers=None, auth=None,
parse_remote_metadata=False, vendor_kwargs=None, headers=None, cookies=None, auth=None,
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be safer to add the new cookies parameter at the end of the list to avoid breaking existing codebases

owslib/wmts.py Outdated
@@ -887,7 +888,7 @@ class WMTSCapabilitiesReader:
"""Read and parse capabilities document into a lxml.etree infoset
"""

def __init__(self, version='1.0.0', url=None, un=None, pw=None, headers=None, auth=None):
def __init__(self, version='1.0.0', url=None, un=None, pw=None, headers=None, cookies=None, auth=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

As above - move new argument to the end

@ashergg
Copy link
Author

ashergg commented May 18, 2025

Hi @geographika, thanks for the comments. I've added a test, and moved the arguments to the end.

@ashergg ashergg requested a review from geographika May 18, 2025 09:30
@geographika
Copy link
Contributor

Thanks @ashergg ! This looks good to me, and +1 on merging. I'll leave open a few days to see if anyone else has any comments.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants