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.

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

Add option to pass other settings to requests lib #13

Merged
merged 1 commit into from
Dec 12, 2019

Conversation

J535D165
Copy link
Owner

@J535D165 J535D165 commented Dec 7, 2019

Solution for #12.

  1. Parameters like proxies, verify and cert can be passed to every request in cbsodata.
  2. Define global values for parameters like proxies, verify and cert with the cbsodata.options.requests setting.
In [1]: import cbsodata                                                                             

In [3]: cbsodata.options.requests['verify'] = "xyz"                                                          

In [4]: cbsodata.options.requests['verify']                                                                  
Out[4]: 'xyz'

In [5]: cbsodata.get_info("82070ENG")                                                               
Out[5]: 
{'ID': 0,
 'Title': 'Caribbean Netherlands; employed labour force characteristics 2012',
 'ShortTitle': 'Caribbean NL; employed 2012',
 'Identifier': '82070ENG',
 'Summary': 'Employed labour force national and international definition\nby sex by position in the labour force and gross monthly income',
 'Modified': '2013-11-28T15:00:00',
....

In [6]: cbsodata.get_info("82070ENG", verify=True)                                                  
Out[6]: 
{'ID': 0,
 'Title': 'Caribbean Netherlands; employed labour force characteristics 2012',
 'ShortTitle': 'Caribbean NL; employed 2012',
 'Identifier': '82070ENG',
 'Summary': 'Employed labour force national and international definition\nby sex by position in the 
...

Copy link

@ties ties left a comment

Choose a reason for hiding this comment

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

I like this feature!

@@ -217,7 +234,7 @@ def _select(select):


def download_data(table_id, dir=None, typed=False, select=None, filters=None,
catalog_url=None, proxies=None):
catalog_url=None, **kwargs):
Copy link

Choose a reason for hiding this comment

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

This API will prevent the usage of kwargs for filters in the future without breaking backwards compatibility (such as in django). Please consider that when making this design choice.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks for this feedback. I think I will implement something like this in the OData4 version #8. This OData3 API deprecates in 2020.

Copy link

Choose a reason for hiding this comment

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

I have a partial implementation of an OData query parser if anyone is interested (grammar + a number of abstract syntax tree manipulations to rewrite it).

Unfortunate it is part of a larger prototype that is not release ready - but ping me if there is interest.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm welcoming all contributions to the OData version 4 API :)

@proxies.setter
def proxies(self, proxies):
warnings.warn(
"Deprecated, use options.requests['proxies'] instead",
Copy link

Choose a reason for hiding this comment

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

Nice forward compatibility 👍

@J535D165
Copy link
Owner Author

@joosbuijsNL can you test this implementation?

@joosbuijsNL
Copy link

@joosbuijsNL can you test this implementation?

Confirmed, this works! Thanks a lot! :)

@J535D165 J535D165 merged commit 0e98738 into master Dec 12, 2019
@J535D165
Copy link
Owner Author

Closes #12

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

Successfully merging this pull request may close these issues.

3 participants