8000 Add flexibility for wkbk/ds id or item in endpoint by jorwoods · Pull Request #570 · tableau/server-client-python · GitHub
[go: up one dir, main page]

Skip to content

Add flexibility for wkbk/ds id or item in endpoint #570

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

Merged
merged 7 commits into from
Apr 8, 2020

Conversation

jorwoods
Copy link
Contributor

This pull request makes a consistent but backwards compatible API for refreshes that accept either the item or it's id for both datasources and workbooks.

Would resolve #562

Copy link
Contributor
@shinchris shinchris left a comment

Choose a reason for hiding this comment

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

@jorwoods thanks for submitting a pull request for this!
Could you add some unit tests and also sign our Contributor License Agreement?

@@ -145,7 +145,8 @@ def update_connection(self, datasource_item, connection_item):
return connection

Copy link
Contributor

Choose a reason for hiding this comment

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

While you're making changes here, can you add the @api(version='2.8') annotation for the refresh method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shinchris I added the API decorator. I didn't see any existing unit tests for the refresh methods that I can expand upon. I need to read a little more about requests_mock to know how to write them appropriately for what this should accomplish.

@jorwoods
Copy link
Contributor Author
jorwoods commented Mar 6, 2020

@shinchris I tried creating a test response XML for the workbook refresh XML, but something is wrong with it. Can someone here take a look and help correct the XML response?

@jorwoods
Copy link
Contributor Author
jorwoods commented Mar 6, 2020

I found the issue with the XML. I copied the response XML from https://help.tableau.com/v2019.3/api/rest_api/en-us/REST/rest_api_ref.htm#update_workbook_now but its missing the closing job tag. The documentation needs to be corrected.

@shinchris
Copy link
Contributor

Thanks @jorwoods!
I've filed a bug with our documentation writer to get that example response fixed.

Have you signed our CLA yet?

@jorwoods
Copy link
Contributor Author

CLA has been signed and emailed in.

@shinchris shinchris merged commit 2552c26 into tableau:development Apr 8, 2020
shinchris pushed a commit that referenced this pull request May 1, 2020
Merge development into master for v0.11 release

v0.11 (1 May 2020)

-Added more fields to Data Acceleration config (#588)
-Added OpenID as an auth setting enum (#610)
-Added support for Data Acceleration Reports (#596)
-Added support for view permissions (#526)
-Materialized views changed to Data Acceleration (#576)
-Improved consistency across workbook/datasource endpoints (#570)
-Fixed print error in update_connection.py (#602)
-Fixed log error in add user endpoint (#608)
@jorwoods jorwoods deleted the jorwoods/wb_ds_item_or_id branch March 25, 2022 12:34
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.

2 participants
0