Skip to content
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

feat(bigquery): expose ProjectID from Client & Job #4076

Closed
wants to merge 2 commits into from

Conversation

derekperkins
Copy link
Contributor

As more and more BigQuery features like BI Engine, Reservations, etc. are opt-in at a project level only, we are now using more dependency injection for multiple clients. As currently written, we have to create a throwaway dataset from the client to expose the project associated with the client and keep track of that separately to map to any jobs. The project is trivially available from both the client and job, so this PR exposes both.

@derekperkins derekperkins requested a review from a team as a code owner May 9, 2021 18:13
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the BigQuery API. label May 9, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 9, 2021
@derekperkins derekperkins changed the title bigquery: expose ProjectID from Client & Job feat(bigquery): expose ProjectID from Client & Job May 9, 2021
@derekperkins
Copy link
Contributor Author

@shollyman @codyoss Any chance this could make it into the 1.19 release?

@codyoss codyoss requested a review from shollyman June 1, 2021 14:23
Copy link
Contributor

@shollyman shollyman left a comment

Choose a reason for hiding this comment

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

Thanks. This seems reasonable to expose. I've also created #4228 for myself to come back through and improve job reference construction.

@shollyman shollyman added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 7, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 7, 2021
@shollyman
Copy link
Contributor

Feedback from others was we should consider whether we should simply change to exported variables. Primary issue is still the retained context; need to take a closer look at this to see if we want to open it up rather than simply exposing functions as laid out here.

@shollyman shollyman added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 8, 2021
@derekperkins
Copy link
Contributor Author

For reference, ProjectID is exported in Dataset. I don't particularly care whether ProjectID is exported or whether it is exposed with a function to protect the integrity of the object. I just worry that taking a closer look will cause this to end up in the backlog.

type Dataset struct {
ProjectID string

shollyman added a commit to shollyman/google-cloud-go that referenced this pull request Jun 24, 2021
PR supersedes: googleapis#4076

Related: googleapis#1294

With this change, project autodetection is enabled via use of a sentinel
value, and the retained project identifier is now exposed on the Client
and Job resources via the Project() function.
gcf-merge-on-green bot pushed a commit that referenced this pull request Jun 24, 2021
…her (#4312)

PR supersedes: #4076

Related: #1294

With this change, project autodetection is enabled via use of a sentinel
value, and the retained project identifier is now exposed on the Client
and Job resources via the Project() function.
@shollyman
Copy link
Contributor

Closing this PR, functionality was added in #4312. Thanks for bringing it to my attention, ended up highlighting the autodetection gap.

@shollyman shollyman closed this Jun 24, 2021
@derekperkins
Copy link
Contributor Author

Thanks for making that happen, much appreciated

@derekperkins derekperkins deleted the patch-2 branch June 24, 2021 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the BigQuery API. cla: yes This human has signed the Contributor License Agreement. do not merge Indicates a pull request not ready for merge, due to either quality or timing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants