External Content Support (Databases and Tables) #445
Conversation
* _PermissionsEndpoint to be used inside Endpoint that has the calls * Added a couple of classes related to permissions, followed naming convention of API
t8y8
changed the title
|
Now that permissions is in, I've rebased this against HEAD. This is much simpler than permissions, though it is more vulnerable to copy/paste errors, so some dedicated eyes are much appreciated. I removed the WIP, but I actually haven't spent as much time on this set, so I'll be testing against a Tableau Catalog enabled Server over the next few days to make sure this is ready. I will also add some tests :) |
|
|
||
| @property | ||
| def remote_type(self): | ||
| return self._remote_type |
There was a problem hiding this comment.
This field should also be defined in the constructor to keep it consistent.
There was a problem hiding this comment.
That's a good point -- what is the pattern here?
That's not a user-editable attribute, and it's something we set purely from the XML response, sometimes we put these in constructors, sometimes we don't.
I can't add it here, just wondering.
| self.parent_srv.namespace) | ||
| return columns | ||
|
|
||
| # Update workbook_connection |
There was a problem hiding this comment.
Copied comment from workbook
| return self._certified | ||
|
|
||
| @certified.setter | ||
| def certified(self, value): |
There was a problem hiding this comment.
This one is missing the boolean property decorator
| self._certification_note = value | ||
|
|
||
| @property | ||
| def metadata_type(self): |
There was a problem hiding this comment.
This one and a few below are not defined in the constructor. Also for port, should it be an integer property?
There was a problem hiding this comment.
Not all of these properties will exist for all types (though metadata_type should).
Right now it lazily adds them based on whatever we pass to _set_values.
We could enumerate them all, but I thought it was a lot of copy/paste work to add them there and to _set_values.
Happy to do it if you think we should. It raises the question again about what makes it into constructor vs what doesn't.
There was a problem hiding this comment.
In my opinion, I think it's good practice to have all properties defined in the constructor. That way we can easily see all the properties that the class can have, and avoids the possibility of an error from accessing it before assigned from the response.
Looks like we've been keeping that pattern for the other models as well.
|
Thanks! Question for you, fixed the small things, and I will create some basic tests for these endpoints, it'll take a bit... lots of xml files to copy/paste :) |
|
Based on the earlier rocket, and letting wait for a week for any other emergency issues,merging this in 🔥. I'll monitor and fix forward any issues |
Release v0.9 ## 0.9 (4 Oct 2019) * Added Metadata API endpoints (#431) * Added site settings for Data Catalog and Prep Conductor (#434) * Added new fields to ViewItem (#331) * Added support and samples for Tableau Server Personal Access Tokens (#465) * Added Permissions endpoints (#429) * Added tags to ViewItem (#470) * Added Databases and Tables endpoints (#445) * Added Flow endpoints (#494) * Added ability to filter projects by topLevelProject attribute (#497) * Improved server_info endpoint error handling (#439) * Improved Pager to take in keyword arguments (#451) * Fixed UUID serialization error while publishing workbook (#449) * Fixed materalized views in request body for update_workbook (#461)
This is a work in progress, and is based on the permissions work, so this PR will fail against dev.Still, here's a preview of the content types coming. They're pretty standard, following existing patterns.The one thing that's a bit new, is I took some pieces of @shinchris 's refactor for objects that have lots of attributes.