Refactor config parsing to allow custom parsers. #412
Merged
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
0golovatyi
approved these changes
Apr 15, 2020
There was a problem hiding this comment.
Great finding!
Since we have now 2 improvements to merge to dev could you also bump up minor version in https://github.com/tableau/TabPy/blob/secureInfo/tabpy/VERSION and update https://github.com/tableau/TabPy/blob/secureInfo/CHANGELOG?
| @@ -15,7 +15,7 @@ def get(self): | |||
| # Some clients may wish to lock down the entire API which can be done through | |||
| # the configuration file. | |||
|
|
|||
| if self.settings[SettingsParameters.AuthInfo] == "true": | |||
| if self.settings[SettingsParameters.AuthInfo] == True: | |||
There was a problem hiding this comment.
Seems like it can be just
if self.settings[SettingsParameters.AuthInfo]:
|
@lriggs The coverage for Python code is very flaky, just ignore that -0.06% decline. |
sbabayan
approved these changes
Apr 15, 2020
0golovatyi
added a commit
that referenced
this pull request
Jul 29, 2020
* added anvoa to supported pre-deployed models in tabpy (#350) * added anvoa to supported pre-deployed models in tabpy * fixed pep8 issue * fixed md * Add Ctrl+C handler (#348) * Add Ctrl+C handler * Fix unit tests warnings for genson * Add test to increase code coverage * Add * Change default from 10Mb to 100Mb for request size * Increase code coverage * Increase code coverage * Convert buffer size to int * Add Ctrl+C test * Delete test added to the wrong folder * Update CHANGELOG * Update test_app.py * Remove dead code * Don't count coverage for multiline expressions * Add test case for invalid protocol * Add test case for _check_endpoint_name * Remove dead code * Fix vulnerabilities found by LGTM (#361) * Fix vulnerabilities found by LGTM * Fix test failures * Dev improvements (#384) * Fix flake8 warnings * Merge from master * Fix pycodestyle * Fix more flake8 warnings * Fix tests to pass again * Create test_coveralls_codestyle.yml (#382) * Use github actions * Documentation improvements (#385) * Delete .travis.yml * Fix Ctrl+C failing on extra parameter in signal handler * Remove outdated documentation for how to configure connection * tabpy.py to use docopt * Update tabpy-user with docopt * Update CHANGELOG * Fix code style * Change regex, add remove method, and edit qeury_timeout (#375) * Added Client.remove method to delete deployed model * Fix bug for query_timeout types * Update CHANGELOG * Fix missing script result/return bug * Fix github workflow for push * Code improvement: app._parse_config (#391) * Update app.py * v0.8.10 (#392) * Collect test coverage with scrutinizer instead of coveralls * Restore coverage collecting with coveralls * Update scrutinizer settings * Add support for Python 3.8 * Fix static page and add unit test for it * Delete obsolete test * Dev cov (#394) * Collect test coverage with scrutinizer instead of coveralls * Restore coverage collecting with coveralls * Update scrutinizer settings * Add support for Python 3.8 * Fix static page and add unit test for it * Delete obsolete test * Restore scrutinizer configuration * Linting as separate build step * Restore scrutinizer configuration * Update .scrutinizer.yml * Update .scrutinizer.yml * Restore scrutinizer configuration * Update pull_request.yml * Code style improvements * Code style improvements * Code style improvements * Add coverall workflow * Initial checkin, working tests. * Made common base class for server info tests. * pep8 checks. * Added documentation for TABPY_AUTH_INFO * Fix spacing. * Refactor config parsing to allow custom parsers. (#412) * Refactor config parsing to allow custom parsers. * Fix pep8 * Update version and changelog. * Changed default for tabpy_auth_info to a boolean. * Secure info (#414) * Update README.md * Doc update (#402) * Fixed broken link * Linked to install doc. * Dev fix spelling (#408) * Add spelling fix workflow * Refactor config parsing to allow custom parsers. * Fix pep8 * Update version and changelog. * Changed default for tabpy_auth_info to a boolean. * Remove configuration for securing the info API and make it secure by default. * Revert "Merge branch 'master' into secureInfo" This reverts commit 3688561, reversing changes made to a4acc65. * Removed auth config from uni test. Co-authored-by: nmannheimer <nathanmannheimer@gmail.com> Co-authored-by: Oleksandr Golovatyi <olekg@outlook.com> * Secure info (#417) * Update README.md * Doc update (#402) * Fixed broken link * Linked to install doc. * Dev fix spelling (#408) * Add spelling fix workflow * Refactor config parsing to allow custom parsers. * Fix pep8 * Update version and changelog. * Changed default for tabpy_auth_info to a boolean. * Remove configuration for securing the info API and make it secure by default. * Revert "Merge branch 'master' into secureInfo" This reverts commit 3688561, reversing changes made to a4acc65. * Removed auth config from uni test. * Removed example for removed setting. * Remove unused example config. Co-authored-by: nmannheimer <nathanmannheimer@gmail.com> Co-authored-by: Oleksandr Golovatyi <olekg@outlook.com> * Clean up API documentation (#420) * Update README.md * Doc update (#402) * Fixed broken link * Linked to install doc. * Dev fix spelling (#408) * Add spelling fix workflow * Update LICENSE update to Tableau Software LLC * v 1.1.0: Secure /info with auth (#415) - Authorization is now required for the /info API method. This method did not check authentication previously. This change is backwards compatible with Tableau clients. - Improved config parsing flexibility. Previously the TABPY_EVALUATE_TIMEOUT setting would be set to a default if tabpy couldn't parse the value. Now it will throw an exception at startup. * Clean up API documentation * Clean up API documentation * Clean up API documentation Co-authored-by: nmannheimer <nathanmannheimer@gmail.com> Co-authored-by: lriggs <lriggs@tableau.com> Co-authored-by: Olek Golovatyi <ogolovatyi@tableau.com> * Code improvements (#431) * Rename tabpy_server and tabpy_tools to server and tools (breaking change) * fix flake8 warnings * Clean up code to reduce number of conditions * Remove pypi publishing instructions - those are Tableau specific * Restore tabpy_tools and tabpy_server names * Restore tabpy_tools and tabpy_server names * Restore tabpy_tools and tabpy_server names * Unit and integration tests passing * Update .gitignore * do not track settings.json for VSCode * Fix server -> tabpy_server * more cleaning for tabpy_server names * make "python setup.py test" work * add coverage module as required * delete tests node for scrutinizer run Co-authored-by: sbabayan <34922408+sbabayan@users.noreply.github.com> Co-authored-by: ogolovatyi <ogolovatyi@OGOLOVATYI.tsi.lan> Co-authored-by: Brennan Bugbee <Bugbeeb@gmail.com> Co-authored-by: Logan Riggs <lriggs@tableau.com> Co-authored-by: Olek Golovatyi <ogolovatyi@tableau.com> Co-authored-by: nmannheimer <nathanmannheimer@gmail.com>
0golovatyi
added a commit
that referenced
this pull request
Aug 14, 2020
* added anvoa to supported pre-deployed models in tabpy (#350) * added anvoa to supported pre-deployed models in tabpy * fixed pep8 issue * fixed md * Add Ctrl+C handler (#348) * Add Ctrl+C handler * Fix unit tests warnings for genson * Add test to increase code coverage * Add * Change default from 10Mb to 100Mb for request size * Increase code coverage * Increase code coverage * Convert buffer size to int * Add Ctrl+C test * Delete test added to the wrong folder * Update CHANGELOG * Update test_app.py * Remove dead code * Don't count coverage for multiline expressions * Add test case for invalid protocol * Add test case for _check_endpoint_name * Remove dead code * Fix vulnerabilities found by LGTM (#361) * Fix vulnerabilities found by LGTM * Fix test failures * Dev improvements (#384) * Fix flake8 warnings * Merge from master * Fix pycodestyle * Fix more flake8 warnings * Fix tests to pass again * Create test_coveralls_codestyle.yml (#382) * Use github actions * Documentation improvements (#385) * Delete .travis.yml * Fix Ctrl+C failing on extra parameter in signal handler * Remove outdated documentation for how to configure connection * tabpy.py to use docopt * Update tabpy-user with docopt * Update CHANGELOG * Fix code style * Change regex, add remove method, and edit qeury_timeout (#375) * Added Client.remove method to delete deployed model * Fix bug for query_timeout types * Update CHANGELOG * Fix missing script result/return bug * Fix github workflow for push * Code improvement: app._parse_config (#391) * Update app.py * v0.8.10 (#392) * Collect test coverage with scrutinizer instead of coveralls * Restore coverage collecting with coveralls * Update scrutinizer settings * Add support for Python 3.8 * Fix static page and add unit test for it * Delete obsolete test * Dev cov (#394) * Collect test coverage with scrutinizer instead of coveralls * Restore coverage collecting with coveralls * Update scrutinizer settings * Add support for Python 3.8 * Fix static page and add unit test for it * Delete obsolete test * Restore scrutinizer configuration * Linting as separate build step * Restore scrutinizer configuration * Update .scrutinizer.yml * Update .scrutinizer.yml * Restore scrutinizer configuration * Update pull_request.yml * Code style improvements * Code style improvements * Code style improvements * Add coverall workflow * Initial checkin, working tests. * Made common base class for server info tests. * pep8 checks. * Added documentation for TABPY_AUTH_INFO * Fix spacing. * Refactor config parsing to allow custom parsers. (#412) * Refactor config parsing to allow custom parsers. * Fix pep8 * Update version and changelog. * Changed default for tabpy_auth_info to a boolean. * Secure info (#414) * Update README.md * Doc update (#402) * Fixed broken link * Linked to install doc. * Dev fix spelling (#408) * Add spelling fix workflow * Refactor config parsing to allow custom parsers. * Fix pep8 * Update version and changelog. * Changed default for tabpy_auth_info to a boolean. * Remove configuration for securing the info API and make it secure by default. * Revert "Merge branch 'master' into secureInfo" This reverts commit 3688561, reversing changes made to a4acc65. * Removed auth config from uni test. Co-authored-by: nmannheimer <nathanmannheimer@gmail.com> Co-authored-by: Oleksandr Golovatyi <olekg@outlook.com> * Secure info (#417) * Update README.md * Doc update (#402) * Fixed broken link * Linked to install doc. * Dev fix spelling (#408) * Add spelling fix workflow * Refactor config parsing to allow custom parsers. * Fix pep8 * Update version and changelog. * Changed default for tabpy_auth_info to a boolean. * Remove configuration for securing the info API and make it secure by default. * Revert "Merge branch 'master' into secureInfo" This reverts commit 3688561, reversing changes made to a4acc65. * Removed auth config from uni test. * Removed example for removed setting. * Remove unused example config. Co-authored-by: nmannheimer <nathanmannheimer@gmail.com> Co-authored-by: Oleksandr Golovatyi <olekg@outlook.com> * Clean up API documentation (#420) * Update README.md * Doc update (#402) * Fixed broken link * Linked to install doc. * Dev fix spelling (#408) * Add spelling fix workflow * Update LICENSE update to Tableau Software LLC * v 1.1.0: Secure /info with auth (#415) - Authorization is now required for the /info API method. This method did not check authentication previously. This change is backwards compatible with Tableau clients. - Improved config parsing flexibility. Previously the TABPY_EVALUATE_TIMEOUT setting would be set to a default if tabpy couldn't parse the value. Now it will throw an exception at startup. * Clean up API documentation * Clean up API documentation * Clean up API documentation Co-authored-by: nmannheimer <nathanmannheimer@gmail.com> Co-authored-by: lriggs <lriggs@tableau.com> Co-authored-by: Olek Golovatyi <ogolovatyi@tableau.com> * Code improvements (#431) * Rename tabpy_server and tabpy_tools to server and tools (breaking change) * fix flake8 warnings * Clean up code to reduce number of conditions * Remove pypi publishing instructions - those are Tableau specific * Restore tabpy_tools and tabpy_server names * Restore tabpy_tools and tabpy_server names * Restore tabpy_tools and tabpy_server names * Unit and integration tests passing * Update .gitignore * do not track settings.json for VSCode * Fix server -> tabpy_server * more cleaning for tabpy_server names * make "python setup.py test" work * add coverage module as required * delete tests node for scrutinizer run * Update postman collection * remove print from error handling code (#439) * remove print from error handling code * remove & for linux cmd Co-authored-by: Olek Golovatyi <ogolovatyi@tableau.com> * Return HTTP 400 status when receiving a request with authentication credentials and authN is not configured (#440) * added 400 bad request response to event when authorization is not set up but user sends username and password * added unit tests to test what happens when there are credentials, but no authentication is required * renamed not_authorized flag to authentication_error flag * changed line formatting * changed the way auth error is handled * Version to 2.0.0 * Version to 2.0.0 * Fix codystyle warnings * Restore scrutinizer settings * Fix codestyle Co-authored-by: sbabayan <34922408+sbabayan@users.noreply.github.com> Co-authored-by: ogolovatyi <ogolovatyi@OGOLOVATYI.tsi.lan> Co-authored-by: Brennan Bugbee <Bugbeeb@gmail.com> Co-authored-by: Logan Riggs <lriggs@tableau.com> Co-authored-by: Olek Golovatyi <ogolovatyi@tableau.com> Co-authored-by: nmannheimer <nathanmannheimer@gmail.com> Co-authored-by: harold-xi <69485232+harold-xi@users.noreply.github.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.
ConfigParser has some helpful functions to parse booleans and ints/floats. The boolean parser handles casing and allows true, 1, yes for true values. This is more relable and user friendly than relying on a direct string match. Updated tests.
One change here is with the TABPY_EVALUATE_TIMEOUT setting. Previously it would try and convert that setting to a float and if it failed would default to 30 seconds. Now it will throw an exception at startup and the process will halt. The new method does handle int -> float conversions.
I could add some extra code to restore the old behavior, but maybe its better this way since the user will know that they have a setting which isn't being used. Mostly silent defaults can be hard to recognize.