8000 Auto Paging for Users in Groups by t8y8 · Pull Request #204 · tableau/server-client-python · GitHub
[go: up one dir, main page]

Skip to content

Auto Paging for Users in Groups #204

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
Jul 5, 2017
Merged

Auto Paging for Users in Groups #204

merged 7 commits into from
Jul 5, 2017

Conversation

t8y8
Copy link
Collaborator
@t8y8 t8y8 commented Jun 29, 2017

Subsumes #147 because the git history got all weird.

I still need to make the tests work properly, but a manual test indicates this all works well.

/cc @cmtoomey who may want to try this out

@@ -59,10 +73,6 @@ def remove_user(self, group_item, user_id):
self._remove_user(group_item, user_id)
try:
users = group_item.users
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is actually unnecessary because every call is a new Pager now, but we could return the list from add_user or we can just remove this from the call enitrely

Copy link
Contributor
@graysonarts graysonarts left a comment

Choose a reason for hiding this comment

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

Can you show me an example of usage?

@t8y8
Copy link
Collaborator Author
t8y8 commented Jun 30, 2017

Usage:

#  get a list of users for each group
mapping = {}

groups = TSC.Pager(server.groups)
for group in groups:
    server.groups.populate_users(group)
    mapping[group.name] = group.users

# get users for one group, loop and do something
server.groups.populate_users(group)
my_users = group.users
for u in my_users:
    do_the_thing(my_user)

# add/delete
server.groups.populate_users(group)
start = set(group.users)
server.groups.add_user([u1, u2, u3[)
end = server.groups.user
print(str(end - start))

t8y8 added 2 commits June 30, 2017 13:43
@@ -57,28 +71,12 @@ def create(self, group_item):
@api(version="2.0")
def remove_user(self, group_item, user_id):
self._remove_user(group_item, user_id)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At this point it may not even be necessary to have the _remove_user function since we don't do anything special now

logger.info('Removed user (id: {0}) from group (ID: {1})'.format(user_id, group_item.id))

# Adds 1 user to 1 group
@api(version="2.0")
def add_user(self, group_item, user_id):
new_user = self._add_user(group_item, user_id)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ditto

@t8y8
Copy link
Collaborator Author
t8y8 commented Jul 5, 2017

@RussTheAerialist

Here's a cleaned up rev -- I've been testing manually for a bit and seems to work the way I think it should.

I'd like to check this in and get @cmtoomey and @ugamarkj to check it out. If they like it, I can generalize it (I've got an idea to refactor it with passing callables all the way down) to other sub-models, whichever feels easier to maintain.

Copy link
Contributor
@graysonarts graysonarts left a comment

Choose a reason for hiding this comment

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

Small minor nitpick but otherwise good 🚀

if hasattr(endpoint, 'get'):
# The simpliest case is to take an Endpoint and call its get
self._endpoint = endpoint.get
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

Elif callable(...):
...
Else:
raise ...

(Sorry I'm on my phone, didn't bring a computer with me to California)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh good call, a little cleaner!

@t8y8 t8y8 merged commit c6ab7da into tableau:development Jul 5, 2017
@t8y8 t8y8 deleted the 141-redux branch July 5, 2017 19:21
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