Skip to content

fix: send BigQuery cancel request when canceling bigframes process #103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Oct 12, 2023
Merged

fix: send BigQuery cancel request when canceling bigframes process #103

merged 3 commits into from
Oct 12, 2023

Conversation

milkshakeiii
Copy link
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

@milkshakeiii milkshakeiii requested review from a team as code owners October 12, 2023 15:31
@product-auto-label product-auto-label bot added size: s Pull request size is small. api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. labels Oct 12, 2023
@milkshakeiii milkshakeiii changed the title feat: send BigQuery cancel request when canceling bigframes process fix: send BigQuery cancel request when canceling bigframes process Oct 12, 2023
@milkshakeiii milkshakeiii merged commit e325fbb into main Oct 12, 2023
@milkshakeiii milkshakeiii deleted the b303950904-job-cancelation branch October 12, 2023 21:15
@release-please release-please bot mentioned this pull request Oct 12, 2023
Merged
shobsi
shobsi reviewed Oct 13, 2023
View reviewed changes
Copy link
Contributor

@shobsi shobsi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this change, this will go a long way for the notebook experience!

bigframes/formatting_helpers.py
@@ -153,6 +153,11 @@ def wait_for_query_job(
except api_core_exceptions.GoogleAPICallError as exc:
add_feedback_link(exc)
raise
except KeyboardInterrupt:
query_job.cancel()
print(f"Requested cancelation for {query_job.job_type} {query_job.job_id}...")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda nitpick - English vocab is not my forte but I wondered about "cancelation" vs "cancellation" and looks like in googleapis repos the former is a rarity, the latter is commonly used.

bigframes/formatting_helpers.py
@@ -190,6 +195,11 @@ def wait_for_job(job: GenericJob, progress_bar: Optional[str] = None):
except api_core_exceptions.GoogleAPICallError as exc:
add_feedback_link(exc)
raise
except KeyboardInterrupt:
job.cancel()
print(f"Requested cancelation for {job.job_type} {job.job_id}...")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may like to follow the following pattern done elsewhere for consistency:

  1. job_type.capitalize() like in get_base_job_loading_*
  2. "<job_type> job <job_id>" instead of "<job_type> <job_id>", like in get_*_job_loading_*

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Shobhit, created #111 to address these.

bigframes/formatting_helpers.py
@@ -153,6 +153,11 @@ def wait_for_query_job(
except api_core_exceptions.GoogleAPICallError as exc:
add_feedback_link(exc)
raise
except KeyboardInterrupt:
query_job.cancel()
print(f"Requested cancelation for {query_job.job_type} {query_job.job_id}...")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda nitpick - English vocab is not my forte but I wondered about "cancelation" vs "cancellation" and looks like in googleapis repos the former is a rarity, the latter is commonly used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Shobhit, created #111 to address these.

gcf-merge-on-green bot pushed a commit that referenced this pull request Oct 18, 2023
@release-please
chore(main): release 0.9.0 (#108)
e2788a8 
🤖 I have created a release *beep* *boop*
---


## [0.9.0](https://togithub.com/googleapis/python-bigquery-dataframes/compare/v0.8.0...v0.9.0) (2023-10-18)


### ⚠ BREAKING CHANGES

* rename `bigframes.pandas.reset_session` to `close_session` ([#101](https://togithub.com/googleapis/python-bigquery-dataframes/issues/101))

### Features

* Add `bigframes.options.bigquery.application_name` for partner attribution ([#117](https://togithub.com/googleapis/python-bigquery-dataframes/issues/117)) ([52d64ff](https://togithub.com/googleapis/python-bigquery-dataframes/commit/52d64ffdbbab16b1d94974b543ce9080be1ec0d1))
* Add AtIndexer getitems ([#107](https://togithub.com/googleapis/python-bigquery-dataframes/issues/107)) ([752b01f](https://togithub.com/googleapis/python-bigquery-dataframes/commit/752b01ff9df114c54ed58eb96956e9ce34a8ed47))
* Rename `bigframes.pandas.reset_session` to `close_session` ([#101](https://togithub.com/googleapis/python-bigquery-dataframes/issues/101)) ([36693bf](https://togithub.com/googleapis/python-bigquery-dataframes/commit/36693bff398c23e179d9bde95d52cbaddaf85c45))
* Send BigQuery cancel request when canceling bigframes process ([#103](https://togithub.com/googleapis/python-bigquery-dataframes/issues/103)) ([e325fbb](https://togithub.com/googleapis/python-bigquery-dataframes/commit/e325fbb1c91e040d87df10f7d4d5ce53f7c052cb))
* Support external packages in `remote_function` ([#98](https://togithub.com/googleapis/python-bigquery-dataframes/issues/98)) ([ec10c4a](https://togithub.com/googleapis/python-bigquery-dataframes/commit/ec10c4a5a7833c42e28fe9e7b734bc0c4fb84b6e))
* Use ArrowDtype for STRUCT columns in `to_pandas` ([#85](https://togithub.com/googleapis/python-bigquery-dataframes/issues/85)) ([9238fad](https://togithub.com/googleapis/python-bigquery-dataframes/commit/9238fadcfa7e843be6564813ff3131893b79f8b0))


### Bug Fixes

* Support multiindex for three loc getitem overloads ([#113](https://togithub.com/googleapis/python-bigquery-dataframes/issues/113)) ([68e3cd3](https://togithub.com/googleapis/python-bigquery-dataframes/commit/68e3cd37258084d045ea1075e5e61df12c28faac))


### Performance Improvements

* If primary keys are defined, `read_gbq` avoids copying table data ([#112](https://togithub.com/googleapis/python-bigquery-dataframes/issues/112)) ([e6c0cd1](https://togithub.com/googleapis/python-bigquery-dataframes/commit/e6c0cd1777736e0fa7285da59625fbac487573bd))


### Documentation

* Add documentation for `Series.struct.field` and `Series.struct.explode` ([#114](https://togithub.com/googleapis/python-bigquery-dataframes/issues/114)) ([a6dab9c](https://togithub.com/googleapis/python-bigquery-dataframes/commit/a6dab9cdb7dd0e56c93ca96b665ab1be1baac5e5))
* Add open-source link in API doc ([#106](https://togithub.com/googleapis/python-bigquery-dataframes/issues/106)) ([db51fe3](https://togithub.com/googleapis/python-bigquery-dataframes/commit/db51fe340f644a0d7c911c11d92c8299a4be3446))
* Update ML overview API doc ([#105](https://togithub.com/googleapis/python-bigquery-dataframes/issues/105)) ([1b3f3a5](https://togithub.com/googleapis/python-bigquery-dataframes/commit/1b3f3a5374915b2833c6c1ac05670e9708f07bff))

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shobsi shobsi shobsi left review comments

@TrevorBergeron TrevorBergeron TrevorBergeron approved these changes

Assignees
No one assigned
Labels
api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. size: s Pull request size is small.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@milkshakeiii @TrevorBergeron @shobsi