Skip to content

fix: Do not set job timeout extra property if None#1987

Merged
Linchin merged 2 commits intogoogleapis:mainfrom
bmwant:bmwant/fix-job-timeout-none
Aug 19, 2024
Merged

fix: Do not set job timeout extra property if None#1987
Linchin merged 2 commits intogoogleapis:mainfrom
bmwant:bmwant/fix-job-timeout-none

Conversation

@bmwant
Copy link
Contributor

@bmwant bmwant commented Aug 5, 2024 •
edited
Loading

Description

job_timeout_ms declared as optional (see docs), but setting it as None results in error:

google.api_core.exceptions.BadRequest: 400 POST https://bigquery.googleapis.com/bigquery/v2/projects/my-bq-project/jobs?prettyPrint=false: Invalid value at 'job.configuration.job_timeout_ms.value' (TYPE_INT64), "None"

Location: None
Job ID: 5f5bf5bd-dc77-7eda-a2d3-69420a6bf6d6
 [{'@type': 'type.googleapis.com/google.rpc.BadRequest', 'fieldViolations': [{'field': 'job.configuration.job_timeout_ms.value', 'description': 'Invalid value at \'job.configuration.job_timeout_ms.value\' (TYPE_INT64), "None"'}]}]

because of allowing None, but not handling it properly. This fix doesn't cast None to a string in setter. Getter still works as expected, but we do not send invalid value in remote call.

Simple script to reproduce

from typing import Optional

from google.cloud import bigquery

query = "SELECT * FROM `project.table` LIMIT 1"
client = bigquery.Client()
config = bigquery.QueryJobConfig(use_query_cache=True, allow_large_results=True)
my_timeout: Optional[int] = None
config.job_timeout_ms = my_timeout
client.query(query, job_config=config)

@bmwant bmwant requested a review from a team August 5, 2024 14:35
@bmwant bmwant requested a review from a team as a code owner August 5, 2024 14:35
@bmwant bmwant requested a review from PhongChuong August 5, 2024 14:35
@google-cla
Copy link

google-cla bot commented Aug 5, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@conventional-commit-lint-gcf
Copy link

conventional-commit-lint-gcf bot commented Aug 5, 2024 •
edited
Loading

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. api: bigquery Issues related to the googleapis/python-bigquery API. labels Aug 5, 2024
@bmwant bmwant changed the title Allow setting job_timeout_ms to None Do not set job timeout extra property if None Aug 5, 2024
@bmwant bmwant force-pushed the bmwant/fix-job-timeout-none branch from d4a5eaf to f76a4fe Compare August 5, 2024 15:05
Linchin
Linchin reviewed Aug 6, 2024
View reviewed changes
@Linchin Linchin assigned Linchin and unassigned GaoleMeng Aug 6, 2024
@bmwant bmwant force-pushed the bmwant/fix-job-timeout-none branch from f76a4fe to 431802f Compare August 8, 2024 11:57
@product-auto-label product-auto-label bot added size: s Pull request size is small. and removed size: xs Pull request size is extra small. labels Aug 8, 2024
@bmwant
Copy link
Contributor Author

bmwant commented Aug 8, 2024

@Linchin updated, could you review?

@bmwant bmwant force-pushed the bmwant/fix-job-timeout-none branch from f2e77dc to b30ca9f Compare August 12, 2024 18:51
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Aug 12, 2024
@bmwant bmwant force-pushed the bmwant/fix-job-timeout-none branch 2 times, most recently from 07ca2d2 to 602c0b5 Compare August 12, 2024 18:55
@product-auto-label product-auto-label bot added size: s Pull request size is small. and removed size: m Pull request size is medium. labels Aug 12, 2024
@Linchin Linchin added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 13, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 13, 2024
@Linchin Linchin changed the title Do not set job timeout extra property if None fix: Do not set job timeout extra property if None Aug 13, 2024
@Linchin Linchin added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 13, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 13, 2024
@bmwant bmwant force-pushed the bmwant/fix-job-timeout-none branch from 786a5ef to 6417497 Compare August 14, 2024 15:11
@bmwant
Copy link
Contributor Author

bmwant commented Aug 14, 2024

@Linchin rebased on the latest main branch

@Linchin Linchin added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 14, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 14, 2024
@Linchin Linchin added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 15, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 15, 2024
@Linchin
Copy link
Contributor

Linchin commented Aug 15, 2024 •
edited
Loading

Thank you for making the contribution and fix this bug for us! I wonder if you would like to lint the code, as this seems to be the only test that's failing. I haven't had the time to do it. To lint you need to install nox (pip install nox) and run nox -s lint in the repo.

@bmwant bmwant force-pushed the bmwant/fix-job-timeout-none branch from e73a9ae to 166ca14 Compare August 18, 2024 11:37
@bmwant
Copy link
Contributor Author

bmwant commented Aug 18, 2024

Fixed linting, you can re-run checks now

@Linchin Linchin added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 19, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 19, 2024
Linchin
Linchin approved these changes Aug 19, 2024
View reviewed changes
Copy link
Contributor

@Linchin Linchin left a comment

Choose a reason for hiding this comment

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

LGTM

@Linchin Linchin merged commit edcb79c into googleapis:main Aug 19, 2024
@release-please release-please bot mentioned this pull request Aug 19, 2024
Merged
@bmwant bmwant deleted the bmwant/fix-job-timeout-none branch August 19, 2024 18:20
This was referenced Oct 1, 2024
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Linchin Linchin Linchin approved these changes

@PhongChuong PhongChuong Awaiting requested review from PhongChuong

Assignees

@Linchin Linchin

Labels

api: bigquery Issues related to the googleapis/python-bigquery API. size: s Pull request size is small.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@bmwant @Linchin @GaoleMeng @yoshi-kokoro