Moving JSON_OUTPUT_FORMAT outside of _api_request #196
Conversation
This file contains hidden or 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
ericf-firebolt
approved these changes
Aug 19, 2022
Contributor
ericf-firebolt
left a comment
ericf-firebolt
left a comment
There was a problem hiding this comment.
Yeah, this is much cleaner. Don't remember why I decided against it when I made the changes. 🤷♀️
ericf-firebolt
added a commit
that referenced
this pull request
Aug 24, 2022
) * Added async_execution to async_db/cursor and db/cursor. Added an error, . Added a slot and getter for query_id to BaseCursor. (Temporarily?) edited setup.cfg to ignore flake8 C901: function is too complex. * Removed ignore C901 from flake8 settings in setup.cfg. * Fixed a couple of missing arguments in async_db/cursor.py on execute and execute_many calls. * mypy and black cleanup. * Removed set_parameters argument from all(?) functions. Started adding logic for SET async_execution. * Added a bunch of callbacks to cursor tests. * Pulled out a couple more set_parameters variables from function signatures. Edited callbacks and various tiny things in async test_cursor.py. * Added, and commented out, server_side_async_url to unit/conftests.py. * Removed test_set_parameters() from tests/async/cursor.py. * Removed test_set_parameters() from tests/async/cursor.py. * Added ability to see which exectute is failing (execute or executemany) on a unit test run. * Added more explicit error messages to async and sync test_cursor.py modules. * Added some periods. * Replaced cursor reset call in async _do_execute(). * Updated query/message tuple decomposition in test_cursor to be more human-readable. * Fixed a typo and function signature for db/test_cursor_server_side_async_execute() to remove async. * Removed second hard-coding of query_id in server-side async id callback. * Needed to add an await to an _api_request() call. * Used InternalError to error out on no response to async server-side query. Changed AsyncExecutionUnavailableError to generically accept messages. * Added additional checks on rowcount and description in test_cursor_server_side_async_execute(). * Added QueryResponse class. * Minor changes requested on PR. * Added an OperationalError is asynchronous query response is missing query_id. * Had a typo. * Added a warning if asyc_execution is set via a SET parameter rather than being sent in as an argument to execute(). Moved set parameter validation to its own function to deal with flake8 complaints re _do_exectute() being too complex. * Started adding test_cursor_async_execute_error(). * Updated query_url argument in test_cursor_async_execute_error(). * Added AsyncExecutionUnavailableError on server-side async query execution for multi-statement queries. * Seem to have dealt with auth issues in test_cursor_async_execute_error, but getting invalid set parameter on use_standard_sql. Also added # noqa: C901 to parse_type() definition in _types.py because flake8 was suddenly freaking out about it. * Added all necessary set params to url string in test_cursor_async_execute_error(). * Cleaned up string input in test_cursor_async_execute_error(). * Now no token error. * Multi-statement queries now error out correctly. * Reworked a string to try to get commit/push to work. * Had to add an extra auth callback to get all cursor.execute() calls to work. All error tests for async_execution should now be tested correctly. * Removed some parameters from various fns in unit/async_db/test_cursor that I noticed were extraneous. Too bad mypy or flake8 isn't catching these :-/. * Added error check for missing query_id on async_execution. A little bit of cleanup, also Black seems to have made a change or two. * Fixed error for empty response.json on asynch execution. Also changed the use of SET async_execution to an error and included test for that error. * Fixed error for empty response.json on asynch execution. Also changed the use of SET async_execution to an error and included test for that error. * Fixed error for empty response.json on asynch execution. Also changed the use of SET async_execution to an error and included test for that error. * Added a test to check that an server-side asynchronous execution returns a string, as a non-sync-execution query would return rowcount as an int. * Added an integration test to check that an server-side asynchronous execution returns a string, as a non-sync-execution query would return rowcount as an int.~ * Added cancel() to async/cursor.py. Also fixed an error where I was getting empty query ids back from server-side async exectutions, and added an error for that case. * Forgot that I'd commented out most of test_cursor.py. * Trying to get rid of coroutine 'BaseCursor.execute' was never awaited warning. Not yet successful. * Added unit tests for cancel and cancel errors. * Fixed a mistake that would have failed the cancel() integration test. * Fixed several imports that had disappeared (maybe during a merge?). Also fixed an error in test_ss_async_execution_cancel(). * get_status() and two unit tests are added. Integration test is failing with json that has correct field names but empty fields. * Added a new QueryStatus, NOT_AVAILABLE, because checking status will return empty result the first few times. Fixed some issues with the unit tests and updated the integration tests for get_status(). * Added a comment. * Updated a comment. * Added stub fn for async execution fetch. * Keep forgetting to uncomment test code and the pre-commit checks are removing imports. Left in some extra calls to time() and sleep() for now. * Removed some extraneous testing code. * Updated test_ss_async_execution_get_status() after Yoni pointed out that DDL operations will always return empty JSON. Now using an INSERT instead. * Had to comment out test_ss_async_execution_get_status(), as it basically entered an infinite loop. * Added ability to specify output_format in _api_request(), as status requests will fail if it is set. * First set of requested changes on PR. * Removed noqa on _do_execute(). * Renamed _find_async_problems() to _validate_ss_async_settings(). Removed test_cursor_server_side_async_cancel_error from integration tests. * Moved call to _validate_ss_async_settings() into try. * Added asyncio_mode=auto to pytest config in config.cfg, because I was tired of the continual warnings from pytest. * Changed long query in test_queries_async integration tests. Paused execution of after cancel() to ensure I pick up the correct status message. Now sending output_format= on some calls to _api_request(). * Updated all unit tests that test SET parameters to not have output_format in the url. * Changed query_loop() in integration tests/async/test_queries to check for more than one status before exiting the loop. * Noticed that test_anyio_backend_import_issue() was commented out in sync/test_queries.py. That was done bc it won't run on my laptop, but it shouldn't have been committed that way. * Added query tests to integration/dbapi/sync/test_queries.py. Changed async_exectution test to not count SET statements as queries when determining whether a query is multi-statment. Trying to get sleepEachRow() to work for long aync execution queries. * Added query tests to integration/dbapi/sync/test_queries.py. Changed async_execution test to not count SET statements as queries when determining whether a query is multi-statment. Trying to get sleepEachRow() to work for long aync execution queries. * Now errors out when use_standard_sql=0 rather than when it equals 1. This is because if it's off no log entries are written to query_history. Still using a long insert for integration tests on server-side async queries. Added and edited to unit tests for use_standard_sql correctness. * Changed order of synchronous unit tests to move all server-side async tests to end. * Changed order of asynchronous cursor unit tests to move all server-side async tests to end. * Reordered integration and unit test modules to move all server-side async tests to end of modules to facilitate merging main. * Moving JSON_OUTPUT_FORMAT outside of _api_request (#196) * Updated docs to include information on server-side async query execution. * Updated external table mention in comments and removed sentence in docs. Updated dictionary update in _api_request() to make mypy happy. * Made a change to server-side execution explanation for clarity and to explain usefullness of that functionality. * Renamed a function and moved table create and drop out of test_queries.py and into conftest.py. Currently name not defined error. Maybe it's in the wrong conftest file? Co-authored-by: Petro Tiurin <93913847+ptiurin@users.noreply.github.com>
FireboltCorey
pushed a commit
that referenced
this pull request
Aug 29, 2022
* Added async_execution to async_db/cursor and db/cursor. Added an error, . Added a slot and getter for query_id to BaseCursor. (Temporarily?) edited setup.cfg to ignore flake8 C901: function is too complex. * Removed ignore C901 from flake8 settings in setup.cfg. * Fixed a couple of missing arguments in async_db/cursor.py on execute and execute_many calls. * mypy and black cleanup. * Removed set_parameters argument from all(?) functions. Started adding logic for SET async_execution. * Added a bunch of callbacks to cursor tests. * Pulled out a couple more set_parameters variables from function signatures. Edited callbacks and various tiny things in async test_cursor.py. * Added, and commented out, server_side_async_url to unit/conftests.py. * Removed test_set_parameters() from tests/async/cursor.py. * Removed test_set_parameters() from tests/async/cursor.py. * Added ability to see which exectute is failing (execute or executemany) on a unit test run. * Added more explicit error messages to async and sync test_cursor.py modules. * Added some periods. * Replaced cursor reset call in async _do_execute(). * Updated query/message tuple decomposition in test_cursor to be more human-readable. * Fixed a typo and function signature for db/test_cursor_server_side_async_execute() to remove async. * Removed second hard-coding of query_id in server-side async id callback. * Needed to add an await to an _api_request() call. * Used InternalError to error out on no response to async server-side query. Changed AsyncExecutionUnavailableError to generically accept messages. * Added additional checks on rowcount and description in test_cursor_server_side_async_execute(). * Added QueryResponse class. * Minor changes requested on PR. * Added an OperationalError is asynchronous query response is missing query_id. * Had a typo. * Added a warning if asyc_execution is set via a SET parameter rather than being sent in as an argument to execute(). Moved set parameter validation to its own function to deal with flake8 complaints re _do_exectute() being too complex. * Started adding test_cursor_async_execute_error(). * Updated query_url argument in test_cursor_async_execute_error(). * Added AsyncExecutionUnavailableError on server-side async query execution for multi-statement queries. * Seem to have dealt with auth issues in test_cursor_async_execute_error, but getting invalid set parameter on use_standard_sql. Also added # noqa: C901 to parse_type() definition in _types.py because flake8 was suddenly freaking out about it. * Added all necessary set params to url string in test_cursor_async_execute_error(). * Cleaned up string input in test_cursor_async_execute_error(). * Now no token error. * Multi-statement queries now error out correctly. * Reworked a string to try to get commit/push to work. * Had to add an extra auth callback to get all cursor.execute() calls to work. All error tests for async_execution should now be tested correctly. * Removed some parameters from various fns in unit/async_db/test_cursor that I noticed were extraneous. Too bad mypy or flake8 isn't catching these :-/. * Added error check for missing query_id on async_execution. A little bit of cleanup, also Black seems to have made a change or two. * Fixed error for empty response.json on asynch execution. Also changed the use of SET async_execution to an error and included test for that error. * Fixed error for empty response.json on asynch execution. Also changed the use of SET async_execution to an error and included test for that error. * Fixed error for empty response.json on asynch execution. Also changed the use of SET async_execution to an error and included test for that error. * Added a test to check that an server-side asynchronous execution returns a string, as a non-sync-execution query would return rowcount as an int. * Added an integration test to check that an server-side asynchronous execution returns a string, as a non-sync-execution query would return rowcount as an int.~ * Added cancel() to async/cursor.py. Also fixed an error where I was getting empty query ids back from server-side async exectutions, and added an error for that case. * Forgot that I'd commented out most of test_cursor.py. * Trying to get rid of coroutine 'BaseCursor.execute' was never awaited warning. Not yet successful. * Added unit tests for cancel and cancel errors. * Fixed a mistake that would have failed the cancel() integration test. * Fixed several imports that had disappeared (maybe during a merge?). Also fixed an error in test_ss_async_execution_cancel(). * get_status() and two unit tests are added. Integration test is failing with json that has correct field names but empty fields. * Added a new QueryStatus, NOT_AVAILABLE, because checking status will return empty result the first few times. Fixed some issues with the unit tests and updated the integration tests for get_status(). * Added a comment. * Updated a comment. * Added stub fn for async execution fetch. * Keep forgetting to uncomment test code and the pre-commit checks are removing imports. Left in some extra calls to time() and sleep() for now. * Removed some extraneous testing code. * Updated test_ss_async_execution_get_status() after Yoni pointed out that DDL operations will always return empty JSON. Now using an INSERT instead. * Had to comment out test_ss_async_execution_get_status(), as it basically entered an infinite loop. * Added ability to specify output_format in _api_request(), as status requests will fail if it is set. * First set of requested changes on PR. * Removed noqa on _do_execute(). * Renamed _find_async_problems() to _validate_ss_async_settings(). Removed test_cursor_server_side_async_cancel_error from integration tests. * Moved call to _validate_ss_async_settings() into try. * Added asyncio_mode=auto to pytest config in config.cfg, because I was tired of the continual warnings from pytest. * Changed long query in test_queries_async integration tests. Paused execution of after cancel() to ensure I pick up the correct status message. Now sending output_format= on some calls to _api_request(). * Updated all unit tests that test SET parameters to not have output_format in the url. * Changed query_loop() in integration tests/async/test_queries to check for more than one status before exiting the loop. * Noticed that test_anyio_backend_import_issue() was commented out in sync/test_queries.py. That was done bc it won't run on my laptop, but it shouldn't have been committed that way. * Added query tests to integration/dbapi/sync/test_queries.py. Changed async_exectution test to not count SET statements as queries when determining whether a query is multi-statment. Trying to get sleepEachRow() to work for long aync execution queries. * Added query tests to integration/dbapi/sync/test_queries.py. Changed async_execution test to not count SET statements as queries when determining whether a query is multi-statment. Trying to get sleepEachRow() to work for long aync execution queries. * Now errors out when use_standard_sql=0 rather than when it equals 1. This is because if it's off no log entries are written to query_history. Still using a long insert for integration tests on server-side async queries. Added and edited to unit tests for use_standard_sql correctness. * Changed order of synchronous unit tests to move all server-side async tests to end. * Changed order of asynchronous cursor unit tests to move all server-side async tests to end. * Reordered integration and unit test modules to move all server-side async tests to end of modules to facilitate merging main. * Moving JSON_OUTPUT_FORMAT outside of _api_request (#196) * Updated docs to include information on server-side async query execution. * Updated external table mention in comments and removed sentence in docs. Updated dictionary update in _api_request() to make mypy happy. * Made a change to server-side execution explanation for clarity and to explain usefullness of that functionality. * Added a few explicit refs. * Renamed a function and moved table create and drop out of test_queries.py and into conftest.py. Currently name not defined error. Maybe it's in the wrong conftest file? * Damn. I merged and there were uncommitted changes. * Removed two typos in docsrc/connecting_and_queries.rst. * Updated the description of both server-side and client-side async in the docs to clarify the differences. * Fixed some comments in async_db/util.py. * Created setup and teardown fixture for creating and dropping test_tbl in integration tests. * Damn. I merged the PR to main and there were uncommitted changes. * Removed two typos in docsrc/connecting_and_queries.rst. * Updated the description of both server-side and client-side async in the docs to clarify the differences. * Created setup and teardown fixture for creating and dropping test_tbl in integration tests. * Reformatted connecting_and_queries.rst. * Reformatted managing_resources.rst and index.rst. * Reformatted code in managring_resources. * Code indentation typo in managring_resources. * Managed to suppress almost all of the warnings when using sphinx-build. Also shortened a bunch of line lengths in the docs for easier diffing and better legibility when not using line wrapping. Co-authored-by: Petro Tiurin <93913847+ptiurin@users.noreply.github.com>
ericf-firebolt
added a commit
that referenced
this pull request
Aug 30, 2022
… was first merged (#199) * Added async_execution to async_db/cursor and db/cursor. Added an error, . Added a slot and getter for query_id to BaseCursor. (Temporarily?) edited setup.cfg to ignore flake8 C901: function is too complex. * Removed ignore C901 from flake8 settings in setup.cfg. * Fixed a couple of missing arguments in async_db/cursor.py on execute and execute_many calls. * mypy and black cleanup. * Removed set_parameters argument from all(?) functions. Started adding logic for SET async_execution. * Added a bunch of callbacks to cursor tests. * Pulled out a couple more set_parameters variables from function signatures. Edited callbacks and various tiny things in async test_cursor.py. * Added, and commented out, server_side_async_url to unit/conftests.py. * Removed test_set_parameters() from tests/async/cursor.py. * Removed test_set_parameters() from tests/async/cursor.py. * Added ability to see which exectute is failing (execute or executemany) on a unit test run. * Added more explicit error messages to async and sync test_cursor.py modules. * Added some periods. * Replaced cursor reset call in async _do_execute(). * Updated query/message tuple decomposition in test_cursor to be more human-readable. * Fixed a typo and function signature for db/test_cursor_server_side_async_execute() to remove async. * Removed second hard-coding of query_id in server-side async id callback. * Needed to add an await to an _api_request() call. * Used InternalError to error out on no response to async server-side query. Changed AsyncExecutionUnavailableError to generically accept messages. * Added additional checks on rowcount and description in test_cursor_server_side_async_execute(). * Added QueryResponse class. * Minor changes requested on PR. * Added an OperationalError is asynchronous query response is missing query_id. * Had a typo. * Added a warning if asyc_execution is set via a SET parameter rather than being sent in as an argument to execute(). Moved set parameter validation to its own function to deal with flake8 complaints re _do_exectute() being too complex. * Started adding test_cursor_async_execute_error(). * Updated query_url argument in test_cursor_async_execute_error(). * Added AsyncExecutionUnavailableError on server-side async query execution for multi-statement queries. * Seem to have dealt with auth issues in test_cursor_async_execute_error, but getting invalid set parameter on use_standard_sql. Also added # noqa: C901 to parse_type() definition in _types.py because flake8 was suddenly freaking out about it. * Added all necessary set params to url string in test_cursor_async_execute_error(). * Cleaned up string input in test_cursor_async_execute_error(). * Now no token error. * Multi-statement queries now error out correctly. * Reworked a string to try to get commit/push to work. * Had to add an extra auth callback to get all cursor.execute() calls to work. All error tests for async_execution should now be tested correctly. * Removed some parameters from various fns in unit/async_db/test_cursor that I noticed were extraneous. Too bad mypy or flake8 isn't catching these :-/. * Added error check for missing query_id on async_execution. A little bit of cleanup, also Black seems to have made a change or two. * Fixed error for empty response.json on asynch execution. Also changed the use of SET async_execution to an error and included test for that error. * Fixed error for empty response.json on asynch execution. Also changed the use of SET async_execution to an error and included test for that error. * Fixed error for empty response.json on asynch execution. Also changed the use of SET async_execution to an error and included test for that error. * Added a test to check that an server-side asynchronous execution returns a string, as a non-sync-execution query would return rowcount as an int. * Added an integration test to check that an server-side asynchronous execution returns a string, as a non-sync-execution query would return rowcount as an int.~ * Added cancel() to async/cursor.py. Also fixed an error where I was getting empty query ids back from server-side async exectutions, and added an error for that case. * Forgot that I'd commented out most of test_cursor.py. * Trying to get rid of coroutine 'BaseCursor.execute' was never awaited warning. Not yet successful. * Added unit tests for cancel and cancel errors. * Fixed a mistake that would have failed the cancel() integration test. * Fixed several imports that had disappeared (maybe during a merge?). Also fixed an error in test_ss_async_execution_cancel(). * get_status() and two unit tests are added. Integration test is failing with json that has correct field names but empty fields. * Added a new QueryStatus, NOT_AVAILABLE, because checking status will return empty result the first few times. Fixed some issues with the unit tests and updated the integration tests for get_status(). * Added a comment. * Updated a comment. * Added stub fn for async execution fetch. * Keep forgetting to uncomment test code and the pre-commit checks are removing imports. Left in some extra calls to time() and sleep() for now. * Removed some extraneous testing code. * Updated test_ss_async_execution_get_status() after Yoni pointed out that DDL operations will always return empty JSON. Now using an INSERT instead. * Had to comment out test_ss_async_execution_get_status(), as it basically entered an infinite loop. * Added ability to specify output_format in _api_request(), as status requests will fail if it is set. * First set of requested changes on PR. * Removed noqa on _do_execute(). * Renamed _find_async_problems() to _validate_ss_async_settings(). Removed test_cursor_server_side_async_cancel_error from integration tests. * Moved call to _validate_ss_async_settings() into try. * Added asyncio_mode=auto to pytest config in config.cfg, because I was tired of the continual warnings from pytest. * Changed long query in test_queries_async integration tests. Paused execution of after cancel() to ensure I pick up the correct status message. Now sending output_format= on some calls to _api_request(). * Updated all unit tests that test SET parameters to not have output_format in the url. * Changed query_loop() in integration tests/async/test_queries to check for more than one status before exiting the loop. * Noticed that test_anyio_backend_import_issue() was commented out in sync/test_queries.py. That was done bc it won't run on my laptop, but it shouldn't have been committed that way. * Added query tests to integration/dbapi/sync/test_queries.py. Changed async_exectution test to not count SET statements as queries when determining whether a query is multi-statment. Trying to get sleepEachRow() to work for long aync execution queries. * Added query tests to integration/dbapi/sync/test_queries.py. Changed async_execution test to not count SET statements as queries when determining whether a query is multi-statment. Trying to get sleepEachRow() to work for long aync execution queries. * Now errors out when use_standard_sql=0 rather than when it equals 1. This is because if it's off no log entries are written to query_history. Still using a long insert for integration tests on server-side async queries. Added and edited to unit tests for use_standard_sql correctness. * Changed order of synchronous unit tests to move all server-side async tests to end. * Changed order of asynchronous cursor unit tests to move all server-side async tests to end. * Reordered integration and unit test modules to move all server-side async tests to end of modules to facilitate merging main. * Moving JSON_OUTPUT_FORMAT outside of _api_request (#196) * Updated docs to include information on server-side async query execution. * Updated external table mention in comments and removed sentence in docs. Updated dictionary update in _api_request() to make mypy happy. * Made a change to server-side execution explanation for clarity and to explain usefullness of that functionality. * Renamed a function and moved table create and drop out of test_queries.py and into conftest.py. Currently name not defined error. Maybe it's in the wrong conftest file? * Damn. I merged and there were uncommitted changes. * Removed two typos in docsrc/connecting_and_queries.rst. * Updated the description of both server-side and client-side async in the docs to clarify the differences. * Created setup and teardown fixture for creating and dropping test_tbl in integration tests. * Damn. I merged the PR to main and there were uncommitted changes. * Removed two typos in docsrc/connecting_and_queries.rst. * Updated the description of both server-side and client-side async in the docs to clarify the differences. * Created setup and teardown fixture for creating and dropping test_tbl in integration tests. * Typo. * Trying to get a long enough query. Now I'm getting parse errors that I can't figure out. * Just skipping integration tests on cancel and get_status(). * Cleaned up two long strings and commented out assert in integration test db/sync/test_queries::test_server_side_async_execution_get_status. Co-authored-by: Petro Tiurin <93913847+ptiurin@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.
No description provided.