-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat(bigquery): allow construction of jobs from other projects #5048
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): allow construction of jobs from other projects #5048
Conversation
Adds a `JobfromProject()` method to allow users to get metadata from a job that was created within another project. Fixes: googleapis#4228
// JobFromProject creates a Job which refers to an existing BigQuery job. The job | ||
// need not have been created by this package, nor does it need to reside within the same | ||
// project or location as the instantiated client. | ||
func (c *Client) JobFromProject(ctx context.Context, projectID, jobID, location string) (j *Job, err error) { |
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.
This is fine to me. Another option would be to have a config struct here and let users selectively override particular parameters than what they have set with their client.
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.
This pattern is used mostly because its in line with how we expose similar out-of-project resources like datasets. There's a minor preposition inconsistency though, in jobs we do JobFrom, but datasets are DatasetInProject. Seemed better to be consistent with the other job constructor names.
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.
Design LGTM. Are there other places that use c.projectID that need to be updated? Might be good to see a unit test or two that verifies when the job project != client project sets the expect HTTP path.
Other callsites that reference the client's projectID directly is job insertion, either the standard jobs.insert or the optimized jobs.query path. There's other things like dataset iterator Datasets(), but under the hood it calls DatasetsInProject() which allows remote enumeration. |
Getting this to a unit test is surprisingly annoying given the lack of intermediate interfaces, so I wrote a small integration test for now. |
…ature As part of the changes for googleapis#5048 one callsite of getJobInternal was missed. Normally this would easily get identified due to the change in signature, but getJobInternal has a set of expected string arguments, followed by variadic string args. This got picked up by integration testing, but I failed to recall that presubmit doesn't run integration tests so it was caught after submit. Mostly this one's a cautionary tale for having a mix of mandatory and variadic functions that share the same type. Fixes: googleapis#5058
…ature (#5059) As part of the changes for #5048 one callsite of getJobInternal was missed. Normally this would easily get identified due to the change in signature, but getJobInternal has a set of expected string arguments, followed by variadic string args. This got picked up by integration testing, but I failed to recall that presubmit doesn't run integration tests so it was caught after submit. Mostly this one's a cautionary tale for having a mix of mandatory and variadic functions that share the same type. Fixes: #5058
Adds a
JobfromProject()
method to allow users to get metadata from ajob that was created within another project.
Fixes: #4228