Cancel BigQuery job if timeout hits #1672
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
Signed-off-by: Matt Delacour <[email protected]>
MattDelac
force-pushed
the
fix_block_until
branch
from
e56ea82 to
61a51e7
Compare
Codecov Report
@@ Coverage Diff @@
## master #1672 +/- ##
==========================================
- Coverage 83.02% 82.89% -0.13%
==========================================
Files 72 72
Lines 6403 6413 +10
==========================================
Hits 5316 5316
- Misses 1087 1097 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
woop
reviewed
Jun 29, 2021
woop
reviewed
Jun 29, 2021
woop
changed the title
Signed-off-by: Matt Delacour <[email protected]>
woop
approved these changes
Jun 29, 2021
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: achals, MattDelac, woop The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Mwad22
pushed a commit
to Mwad22/feast
that referenced
this pull request
Jul 7, 2021
* Cancel BigQuery job if timedout hits Signed-off-by: Matt Delacour <[email protected]> * Fix typo Signed-off-by: Matt Delacour <[email protected]> Signed-off-by: Mwad22 <[email protected]>
feast-ci-bot
pushed a commit
that referenced
this pull request
Jul 8, 2021
…on to include feature view name in feature naming. (#1641) * test Signed-off-by: David Y Liu <[email protected]> Signed-off-by: Mwad22 <[email protected]> * refactored existing tests to test full_feature_names feature on data retreival, added new tests also. Signed-off-by: Mwad22 <[email protected]> * removed full_feature_names usage from quickstart and README to have more simple examples. Resolved failing tests. Signed-off-by: Mwad22 <[email protected]> * Update CHANGELOG for Feast v0.10.8 Signed-off-by: Mwad22 <[email protected]> * GitBook: [master] 2 pages modified Signed-off-by: Mwad22 <[email protected]> * Schema Inferencing should happen at apply time (#1646) * wip1 Signed-off-by: David Y Liu <[email protected]> * just need to do clean up Signed-off-by: David Y Liu <[email protected]> * linted Signed-off-by: David Y Liu <[email protected]> * improve test coverage Signed-off-by: David Y Liu <[email protected]> * changed placement of inference methods in repo_operation apply_total Signed-off-by: David Y Liu <[email protected]> * updated inference method name + changed to void return since it updates in place Signed-off-by: David Y Liu <[email protected]> * fixed integration test and added comments Signed-off-by: David Y Liu <[email protected]> * Made DataSource event_timestamp_column optional Signed-off-by: David Y Liu <[email protected]> Signed-off-by: Mwad22 <[email protected]> * GitBook: [master] 80 pages modified Signed-off-by: Mwad22 <[email protected]> * GitBook: [master] 80 pages modified Signed-off-by: Mwad22 <[email protected]> * Provide descriptive error on invalid table reference (#1627) * Initial commit to catch nonexistent table Signed-off-by: Cody Lin <[email protected]> Signed-off-by: Cody Lin <[email protected]> * simplify nonexistent BQ table test Signed-off-by: Cody Lin <[email protected]> * clean up table_exists exception Signed-off-by: Cody Lin <[email protected]> * remove unneeded variable Signed-off-by: Cody Lin <[email protected]> * function name change to _assert_table_exists Signed-off-by: Cody Lin <[email protected]> * Initial commit to catch nonexistent table Signed-off-by: Cody Lin <[email protected]> Signed-off-by: Cody Lin <[email protected]> * simplify nonexistent BQ table test Signed-off-by: Cody Lin <[email protected]> * clean up table_exists exception Signed-off-by: Cody Lin <[email protected]> * function name change to _assert_table_exists Signed-off-by: Cody Lin <[email protected]> * fix lint errors and rebase Signed-off-by: Cody Lin <[email protected]> * Fix get_table(None) error Signed-off-by: Cody Lin <[email protected]> * custom exception for both missing file and BQ source Signed-off-by: Cody Lin <[email protected]> * revert FileSource checks Signed-off-by: Cody Lin <[email protected]> * Use DataSourceNotFoundException instead of subclassing Signed-off-by: Cody Lin <[email protected]> * Moved assert_table_exists out of the BQ constructor to apply_total Signed-off-by: Cody Lin <[email protected]> * rename test and test asset Signed-off-by: Cody Lin <[email protected]> * move validate logic back to data_source Signed-off-by: Cody Lin <[email protected]> * fixed tests Signed-off-by: Cody Lin <[email protected]> * Set pytest.integration for tests that access BQ Signed-off-by: Cody Lin <[email protected]> * Import pytest in failed test files Signed-off-by: Cody Lin <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Refactor OnlineStoreConfig classes into owning modules (#1649) * Refactor OnlineStoreConfig classes into owning modules Signed-off-by: Achal Shah <[email protected]> * make format Signed-off-by: Achal Shah <[email protected]> * Move redis too Signed-off-by: Achal Shah <[email protected]> * update test_telemetery Signed-off-by: Achal Shah <[email protected]> * add a create_repo_config method that should be called instead of RepoConfig ctor directly Signed-off-by: Achal Shah <[email protected]> * fix the table reference in repo_operations Signed-off-by: Achal Shah <[email protected]> * reuse create_repo_config Signed-off-by: Achal Shah <[email protected]> Remove redis provider reference * CR comments Signed-off-by: Achal Shah <[email protected]> * Remove create_repo_config in favor of __init__ Signed-off-by: Achal Shah <[email protected]> * make format Signed-off-by: Achal Shah <[email protected]> * Remove print statement Signed-off-by: Achal Shah <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Possibility to specify a project for BigQuery queries (#1656) Signed-off-by: Matt Delacour <[email protected]> Co-authored-by: Achal Shah <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Refactor OfflineStoreConfig classes into their owning modules (#1657) * Refactor OfflineStoreConfig classes into their owning modules Signed-off-by: Achal Shah <[email protected]> * Fix error string Signed-off-by: Achal Shah <[email protected]> * Generic error class Signed-off-by: Achal Shah <[email protected]> * Merge conflicts Signed-off-by: Achal Shah <[email protected]> * make the store type work, and add a test that uses the fully qualified name of the OnlineStore Signed-off-by: Achal Shah <[email protected]> * Address comments from previous PR Signed-off-by: Achal Shah <[email protected]> * CR updates Signed-off-by: Achal Shah <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Run python unit tests in parallel (#1652) Signed-off-by: Achal Shah <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Rename telemetry to usage (#1660) * Rename telemetry to usage Signed-off-by: Tsotne Tabidze <[email protected]> * Update docs Signed-off-by: Tsotne Tabidze <[email protected]> * Update .prow and infra Signed-off-by: Tsotne Tabidze <[email protected]> * Rename file Signed-off-by: Tsotne Tabidze <[email protected]> * Change url Signed-off-by: Tsotne Tabidze <[email protected]> * Re-add telemetry.md for backwards-compatibility Signed-off-by: Tsotne Tabidze <[email protected]> Signed-off-by: Mwad22 <[email protected]> * resolved final comments on PR (variable renaming, refactor tests) Signed-off-by: Mwad22 <[email protected]> * reformatted after merge conflict Signed-off-by: Mwad22 <[email protected]> * Update CHANGELOG for Feast v0.11.0 Signed-off-by: Willem Pienaar <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Update charts README (#1659) Adding feast jupyter link to it. + Fix the helm 'feast-serving' name in aws/azure terraform. Signed-off-by: szalai1 <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Added Redis to list of online stores for local provider in providers reference doc. (#1668) Signed-off-by: Nel Swanepoel <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Grouped inferencing statements together in apply methods for easier readability (#1667) * grouped inferencing statements together Signed-off-by: David Y Liu <[email protected]> * update in testing Signed-off-by: David Y Liu <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Add RedshiftDataSource (#1669) * Add RedshiftDataSource Signed-off-by: Tsotne Tabidze <[email protected]> * Call parent __init__ first Signed-off-by: Tsotne Tabidze <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Provide the user with more options for setting the to_bigquery config (#1661) * Provide more options for to_bigquery config Signed-off-by: Cody Lin <[email protected]> * Fix default job_config when none; remove excessive testing Signed-off-by: Cody Lin <[email protected]> * Add param type and docstring Signed-off-by: Cody Lin <[email protected]> * add docstrings and typing Signed-off-by: Cody Lin <[email protected]> * Apply docstring suggestions from code review Co-authored-by: Willem Pienaar <[email protected]> Signed-off-by: Cody Lin <[email protected]> Co-authored-by: Willem Pienaar <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Add streaming sources to the FeatureView API (#1664) * Add a streaming source to the FeatureView API This diff only updates the API. It is currently up to the providers to actually use this information to spin up resources to consume events from the stream sources. Signed-off-by: Achal Shah <[email protected]> * remove stuff from rebase Signed-off-by: Achal Shah <[email protected]> * make format Signed-off-by: Achal Shah <[email protected]> * Update protos Signed-off-by: Achal Shah <[email protected]> * lint Signed-off-by: Achal Shah <[email protected]> * format Signed-off-by: Achal Shah <[email protected]> * CR Signed-off-by: Achal Shah <[email protected]> * fix test Signed-off-by: Achal Shah <[email protected]> * lint Signed-off-by: Achal Shah <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Add to_table() to RetrievalJob object (#1663) * Add notion of OfflineJob Signed-off-by: Matt Delacour <[email protected]> * Use RetrievalJob instead of creating a new OfflineJob object Signed-off-by: Matt Delacour <[email protected]> * Add to_table() in integration tests Signed-off-by: Matt Delacour <[email protected]> Co-authored-by: Tsotne Tabidze <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Rename to_table to to_arrow (#1671) Signed-off-by: Matt Delacour <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Cancel BigQuery job if timeout hits (#1672) * Cancel BigQuery job if timedout hits Signed-off-by: Matt Delacour <[email protected]> * Fix typo Signed-off-by: Matt Delacour <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Fix Feature References example (#1674) Fix Feature References example by passing `entity_rows` to `get_online_features()` Signed-off-by: Mwad22 <[email protected]> * Allow strings for online/offline store instead of dicts (#1673) Signed-off-by: Achal Shah <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Remove default list from the FeatureView constructor (#1679) Signed-off-by: Achal Shah <[email protected]> Signed-off-by: Mwad22 <[email protected]> * made changes requested by @tsotnet Signed-off-by: Mwad22 <[email protected]> * Fix unit tests that got broken by Pandas 1.3.0 release (#1683) Signed-off-by: Tsotne Tabidze <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Add support for DynamoDB and S3 registry (#1483) * Add support for DynamoDB and S3 registry Signed-off-by: lblokhin <[email protected]> * rcu and wcu as a parameter of dynamodb online store Signed-off-by: lblokhin <[email protected]> * fix linter Signed-off-by: lblokhin <[email protected]> * aws dependency to extras Signed-off-by: lblokhin <[email protected]> * FEAST_S3_ENDPOINT_URL Signed-off-by: lblokhin <[email protected]> * tests Signed-off-by: lblokhin <[email protected]> * fix signature, after merge Signed-off-by: lblokhin <[email protected]> * aws default region name configurable Signed-off-by: lblokhin <[email protected]> * add offlinestore config type to test Signed-off-by: lblokhin <[email protected]> * review changes Signed-off-by: lblokhin <[email protected]> * review requested changes Signed-off-by: lblokhin <[email protected]> * integration test for Dynamo Signed-off-by: lblokhin <[email protected]> * change the rest of table_name to table_instance (where table_name is actually an instance of DynamoDB Table object) Signed-off-by: lblokhin <[email protected]> * fix DynamoDBOnlineStore commit Signed-off-by: lblokhin <[email protected]> * move client to _initialize_dynamodb Signed-off-by: lblokhin <[email protected]> * rename document_id to entity_id and Row to entity_id Signed-off-by: lblokhin <[email protected]> * The default value is None Signed-off-by: lblokhin <[email protected]> * Remove Datastore from the docstring. Signed-off-by: lblokhin <[email protected]> * get rid of the return call from S3RegistryStore Signed-off-by: lblokhin <[email protected]> * merge two exceptions Signed-off-by: lblokhin <[email protected]> * For ci requirement Signed-off-by: lblokhin <[email protected]> * remove configuration from test Signed-off-by: lblokhin <[email protected]> * feast-integration-tests for tests Signed-off-by: lblokhin <[email protected]> * change test path Signed-off-by: lblokhin <[email protected]> * add fixture feature_store_with_s3_registry to test Signed-off-by: lblokhin <[email protected]> * region required Signed-off-by: lblokhin <[email protected]> * Address the rest of the comments Signed-off-by: Tsotne Tabidze <[email protected]> * Update to_table to to_arrow Signed-off-by: Tsotne Tabidze <[email protected]> Co-authored-by: Tsotne Tabidze <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Parallelize integration tests (#1684) * Parallelize integration tests Signed-off-by: Tsotne Tabidze <[email protected]> * Update the usage flag Signed-off-by: Tsotne Tabidze <[email protected]> Signed-off-by: Mwad22 <[email protected]> * BQ exception should be raised first before we check the timedout (#1675) Signed-off-by: Matt Delacour <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Update sdk/python/feast/infra/provider.py Co-authored-by: Willem Pienaar <[email protected]> Signed-off-by: Mwad22 <[email protected]> * Update sdk/python/feast/feature_store.py Co-authored-by: Willem Pienaar <[email protected]> Signed-off-by: Mwad22 <[email protected]> * made error logic/messages more descriptive Signed-off-by: Mwad22 <[email protected]> * made error logic/messages more descriptive. Signed-off-by: Mwad22 <[email protected]> * Simplified error messages Signed-off-by: Mwad22 <[email protected]> * ran formatter, issue in errors.py Signed-off-by: Mwad22 <[email protected]> * python linter issues resolved Signed-off-by: Mwad22 <[email protected]> * removed unnecessary default assignment in get_historical_features. default now set only in feature_store.py Signed-off-by: Mwad22 <[email protected]> * added error message assertion for feature name collisions, and other nitpick changes Signed-off-by: Mwad22 <[email protected]> Co-authored-by: David Y Liu <[email protected]> Co-authored-by: Tsotne Tabidze <[email protected]> Co-authored-by: Achal Shah <[email protected]> Co-authored-by: David Y Liu <[email protected]> Co-authored-by: Willem Pienaar <[email protected]> Co-authored-by: codyjlin <[email protected]> Co-authored-by: Matt Delacour <[email protected]> Co-authored-by: Willem Pienaar <[email protected]> Co-authored-by: Peter Szalai <[email protected]> Co-authored-by: Nel Swanepoel <[email protected]> Co-authored-by: Willem Pienaar <[email protected]> Co-authored-by: Greg Kuhlmann <[email protected]> Co-authored-by: Leonid <[email protected]>
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.
Signed-off-by: Matt Delacour [email protected]
What this PR does / why we need it:
The current function
_block_until_done()has a flaw as if the timedout hits (akastop_after_delay(1800)), then the job will still run in BigQuery and won't be cancelled.An alternative would be the possibility to add a timedout argument to the BQ config job but could not find anything useful
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: