-
Notifications
You must be signed in to change notification settings - Fork 436
Bugfix 66 add create group basic #69
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
Conversation
<tsResponse xmlns="http://tableau.com/api" | ||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
xsi:schemaLocation="http://tableau.com/api http://tableau.com/api/ts-api-2.3.xsd"> | ||
<job id="job-id" mode="Asynchronous" type="GroupImport" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is there for future tests? We don't currently let people trigger an async import.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, that's for a future test when I implement import() (see comment above about AD import)
url = self.baseurl | ||
create_req = RequestFactory.Group.create_req(group_item) | ||
server_response = self.post_request(url, create_req) | ||
new_group = GroupItem.from_response(server_response.content)[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what our standard is but we could probably just put "return" here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
We should have a quick chat about what the api should look like for importing an AD group. I don't want to ship something too quickly and then have the signature not work for the future. Besides that ... it looks good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
A request for the tests
response_xml = f.read().decode('utf-8') | ||
with requests_mock.mock() as m: | ||
m.post(self.baseurl, text=response_xml) | ||
group_to_create = TSC.GroupItem('test') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can I be high maintenance and request a non-ascii name in a test as well?
I just remember some bugs in early sample code and libs prototypes around this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, any suggestions on appropriate words or should I just start finding interesting looking utf-8 characters?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got a favorite foreign language?
I used "蚵仔煎" because it's my favorite street food in the entire world...
🚀 |
8b4e59a
to
569df2a
Compare
🚀 🚀 |
Addresses creation of groups for #66 . Importing is not currently supported. |
Enable a prototypical API for creating new connections and datasources from scratch. This uses ET to do XML manipulation, and is hard coded to "10.0" style integrated connections. This will go away with the new editor/physical/logical model work underway, but it fixes a bug and will let us play around for now.
@bryantbhowell - I think you are looking for some ability to create groups. Groups are handled in two separate ways even though it's the same create endpoint in the server.
This handles creating basic groups, but AD group imports are a bit more complicated (because they are done in the background). Do you need AD imports immediately?