[WIP] First run at internal pager#147
Conversation
|
Take two! Thanks to @RussTheAerialist my inability to get the lambda right even though I totally tried it is totally solved -- it works using the internal Pager and is backwards compatible. BUT, there are some issues:
|
| raise MissingRequiredFieldError(error) | ||
|
|
||
| # TODO should this be a list or a Pager directly? | ||
| all_users = list(Pager(lambda options: self._get_users_for_group(group_item, options), req_options)) |
There was a problem hiding this comment.
I really, really, really, really don't like turning this into a list. Imagine a group that has 10,000 users. This will do 100 calls to the server before returning. (or 10, depending on what the default page size is, I think it's 100, but I don't remember)
What if all you wanted to do was users[0]? You'd be fetching way more than you need.
I'd rather break compatibility (since this does actually break backcompat because it doesn't return a pagination object) and called it iter_users() or something.
There was a problem hiding this comment.
Yeah, this was before we spoke the second time.
I'll need to do more rework to update all the old tests to actually test add/remove user properly. Stay tuned.
There was a problem hiding this comment.
ah okay, didn't realize the change was before we talked.
|
lets talk in the office. I don't totally understand. :) |
|
Recording the high level API: Then you can call This is simple, clear, and works with existing APIs (after some adapting that @RussTheAerialist and I worked out). BUT, there's a problem... what do we do with One option is to make GroupItem.users get a new Pager everytime, which could be appealing -- but then our model would have to carry knowledge of the server session somehow, and I'm not sure how that API would go. And that's where I'm stuck. /cc @LGraber @RussTheAerialist I'll try to catch you guys during hack week, but you're so busy :P |
t8y8
force-pushed
the
fix-141-paging-populate
branch
from
50d94bb to
d553b99
Compare
…the use of the new endpoint. added unit tests
…d sample to download_view_image.py. Comments clean up
* GET and POST tests verify headers, body, and query strings coming from `Endpoint`
* GET and POST tests verify headers, body, and query strings coming from `Endpoint`
… sample code. For upcoming 2.6 REST API Version.
…e code. For upcoming 2.6 REST API Version.
…e class that contains shared tagging functionality.
Fixed a few minor nits from PR.
Part two of tableau#125 This backfills all existing APIs with their minimum version. Checking this in early in release cycle for baketime.
* Add a new decorator that checks parameters against the version. Used with the @api decorator. * Add extract_only flags to workbooks and data sources, protected behind v2.5 flag
* Response to code reviews. Put all request options into 1 file. renamed sample to download_view_image.py. Comments clean up * Add api annotation to all current endpoints (tableau#125) Part two of tableau#125 This backfills all existing APIs with their minimum version. Checking this in early in release cycle for baketime. * initial implement of get all and get specific for Extract Refresh Tasks * fixing test failure in the schedule_item code * pep8 fixes * Download with extract_only and parameter checking (tableau#143) * Add a new decorator that checks parameters against the version. Used with the @api decorator. * Add extract_only flags to workbooks and data sources, protected behind v2.5 flag * Correct the path to extract refresh tasks * fixing missed pep8 failure * adding runNow to the interface * fixing pep8 issues * Add header documentation to the sample. * addressing tyler's feedback
* Added a flag to the server object to allow you go use the server version by default * I disliked the 'highest version' I think server version makes more sense. * ctor should use the non-dep function
* initial checkin of auto versioning * fix version tag * fix mistaken version configuration * clean up
* Update changelog and contributors for 0.4 preping v0.4 * reordering by date of contributions
…leau#175) `parameter_added_in` got a facelift and now takes keyword arguments, where the value represents the version for that keyword. It looks a little cleaner and makes the checking code much simpler. Also renamed the `extract_only` to be `no_extract` because it means the opposite of `extract_only`, oops.
* adding to ref pages; saving progress so far * Check in to save progress-o * Another chekin' for the cause * Check in more stuff; added api buckets in docs menu * Updated with users, groups; snapshot * Check in more changes to ref pages, minor fix for docs menu * more updates; formatting fixes, etc. * um updates, workbooks in prog, etc. * getting there, mostly done. * sorted, added filters, requests, etc. * ready for review.... * update to the left nav for new classes * Added sites.get_by_name; added quotes to sever.version number example * updates from tech review; added no_extract to downloads (workbook, data sources) * Added workbooks.update_connections (the cause of merge conflict); fixed some connectionitem info * tech review fixes, removed groups.get_by_id from examples, fixed datasource update example * Ran spellcheck, fixed misc errors
Revision History settings were present in the SiteItem model but not actually serialized and didn't have setters/getters. I've updated that and enhanced the is_int decorator to allow exemptions in the case of sentinels that fall outside the normal range.
* initial infrastructure for smoke tests * trying to fix travis errors by not pinning the version * fix pinning to major version, not minor
* Enble always sorting when using pager because queries are not currently deterministic * I forgot to format the files * Fixing tyler's nit
* Sample group filtering script * Added comments and specific error catching * Renamed the file to follow the naming convention * commented on return type, example group creation, cleaned up code * Better way to call and catch return from rest api * Proper printing instructions
* Added filtering on project names in new sample script * Minor comment change * Clearer unpacking rest api response objects * Proper printing instructions * Style changes, removed_, using call to switch server version
* Support for Certified Data Sources in the REST API
Renames `no_extract` to `include_extract` for a clearer intent on what the parameter does. `no_extract` is kept but the default changes to `None` and we detect if it's used and throw a DeprecationWarning.
fixing doc issue: Workbook API Reference naming convention is off in Example for get() tableau#193 tableau#193
…to mock out a Pager, so I skipped them for this round.
…t-python into fix-141-paging-populate
t8y8
force-pushed
the
fix-141-paging-populate
branch
from
d553b99 to
e904050
Compare
|
Other than the work to get tests working, I think, if we like this (elegant and simple ;) ) approach, I can go and update all sub-lists (views, connections, etc), or do those in a new PR. Thoughts? |
|
Killing this in favor for #204 |
9 participants
This is an attempt to address #141 from @cmtoomey
It totally works, but I don't like it very much and am very very open to alternative suggestions.
I tried partials and other things but ultimately I couldn't quite get all the arg passing to work.
Only one test fails, and that's because I changed the return signature of the
populate_userscall.I manually tested and this works, including setting request options to something custom.
@RussTheAerialist please save me from myself.