-
Notifications
You must be signed in to change notification settings - Fork 322
feat: updates to fastpath query execution #2268
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.
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
Conversation
This PR plumbs in config support for max_slots, which is an allowlisted preview feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one question to cure my curiosity.
Approved.
| if value is not None: | ||
| self._properties["maxSlots"] = str(value) | ||
| else: | ||
| self._properties.pop("maxSlots", None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#QUESTION
What is the impetus for .pop()?
If value is None, delete the key: value pair entirely?
I know we also did it with jobTimeoutMs and I don't get it there either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#QUESTION
What is the impetus for
.pop()?If
valueisNone, delete thekey: valuepair entirely? I know we also did it withjobTimeoutMsand I don't get it there either.
It's the easier one liner. We either need to check for the key existence explicitly, or wrap the delete in a try...except block to handle the cases where you're setting the None value and the key doesn't yet exist in the dict. Happy to swap to one of these if you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR updates query handling to allow base config properties like job timeout, reservation, and a preview max slots field to leverage the faster path (e.g. using jobs.query rather than jobs.insert).