Jac/filter spaces #225
Merged
Jac/filter spaces #225
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Write tests for methods in ds_and_wb class, including a failing test that expects the view_filter to be un-encoded
The logic of the fix is in ds_and_wb:82 Also refactored some methods to get consistent order of args.
anyoung-tableau
approved these changes
Feb 2, 2023
| # only applicable for png | ||
| logger.warn("Height/width arguments not yet implemented in export") | ||
| except AttributeError as ae: | ||
| logger.debug("Unexpected: no user or default value is present: {}".format(ae.__str__())) | ||
| # Always request high-res images | ||
| request_options.image_resolution = "high" | ||
|
|
||
| @staticmethod | ||
| def apply_pdf_options(request_options: TSC.PDFRequestOptions, args, logger): |
There was a problem hiding this comment.
Did you want to reorder the arguments on this method too?
| # only applicable for png | ||
| logger.warn("Height/width arguments not yet implemented in export") | ||
| except AttributeError as ae: | ||
| logger.debug("Unexpected: no user or default value is present: {}".format(ae.__str__())) |
There was a problem hiding this comment.
Is this messaging correct? What is user in this context?
There was a problem hiding this comment.
I don't expect to get here because argparse should set a default value for these options. The message means there is no user-set value or default value
There was a problem hiding this comment.
Ooooh, gotcha. English is hard. I read it as "There is no user" || "There is no default value" :) Maybe change user to user-specified?
There was a problem hiding this comment.
I ended up just deleting it - I realized that if they aren't set, I would have hit this error well before we get here anyway!
| else: # it must be a filter | ||
| DatasourcesAndWorkbooks.apply_filter_value(request_options, value, logger) | ||
| # the REST API doesn't appear to have the option to disambiguate with "Parameter.<fieldname>" |
There was a problem hiding this comment.
Just double checking. The comment says Parameter. but the code uses the plural Parameters.
- removed super defensive exception catching - stopped sending filters for workbooks - CR feedback
jacalata
added a commit
that referenced
this pull request
Jul 28, 2023
Fixes, code enhancements * remove lowercase call while parsing csv for adding users * set user agent (#219) * Add and clean up logging, always include version in logs (#234, #224) * Encoding filter values to handle spaces and special chars in filters (#225) * replace polling code with library call * readme updates (#235) * Read and warn when user sets thumbnail options. (including 'not yet implemented' message for --thumbnail-group) * freeze tsc dependency (#248) New functionality * added new --filter option to pass in un-encoded value for simpler input * Add --url, --include-all/--embedded-datasources for create/refresh/delete extract commands (#227) * Add new command for listing content on the site (#230) * Implement publishing options overwrite/append/replace arguments (including not-yet-implemented message for --replace) (#236) * implement --proxy, --no-proxy, --use-certificate option (#241) * add --token-file option (#243) --------- Co-authored-by: Bhuvnesh Singh <bhuvnesh.singh@salesforce.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #176
Example export:
result contains only two columns for two products.