10BC0 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

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?

logger.info('Updated datasource item (ID: {0} & connection item {1}'.format(datasource_item.id,
connection_item.id))
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
8000
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