Enable `location` when `calling query_all()` job

[MikhailMS]

Fixes #48

Turned out that location field is part of JobReference struct and that one was hard-coded to default when calling query_all() instead of being exposed to the user

I was thinking between changing query_all signature to either

pub fn query_all<'a> (
   &'a self,
   project_id: &'a str,
   location:   &'a str,
   query: JobConfigurationQuery,
   page_size: Option<i32>,
)

or

pub fn query_all<'a> (
   &'a self,
   project_id: &'a str,
   job_reference: Option<JobReference>,
   query: JobConfigurationQuery,
   page_size: Option<i32>,
)

and it seems like first option makes more sense

I'm eager to get this one merged asap, so do let me know if some changes to PR required before that

[lquerel]

Thanks @MikhailMS for this contribution.

The JobReference and Location fields are optional. I would also like to minimize the impact on other users. So, if you don't mind, I would prefer an API change as follows:

• you keep the query_all method as is
• and you introduce a new method called query_all_with_job_ref with a required JobReference parameter

Once this PR updated, I will publish a new version very quickly. Thanks by advance.

[MikhailMS]

@lquerel That sounds reasonable - I also was a bit unsure since the proposed solution is a breaking change (at least in my book)

What do you think about a counter-proposal:

pub fn query_all_with_location<'a> (
   &'a self,
   project_id: &'a str,
   location:   &'a str,
   query: JobConfigurationQuery,
   page_size: Option<i32>,
) 

?

Mainly because JobReference struct only has 3 parameters:

1. project_id, which is already available as project_id
2. job_id, which should be defaulted to ensure it is unique
3. location, which is the only thing we would be adding

so having the whole structure wrapping location doesn't feel right However, if there are cases when project_id of BigQuery can be different to the one for Job call it would make sense to go with

pub fn query_all_with_job_reference<'a> (
   &'a self,
   project_id: &'a str,
   location:   JobReference,
   query: JobConfigurationQuery,
   page_size: Option<i32>,
) 

Thoughts?

[lquerel]

@MikhailMS

Scenarios with a different project id exist but are probably not so common. I'm ok with your proposal query_all_with_location. I will probably add a query_all_with_job_reference to cover all the scenarios.

[MikhailMS]

@lquerel Sounds awesome - I got those two ready, just need to push to Git - should be good for a review later today :)

[MikhailMS]

@lquerel changes are pushed - though I am not sure that current query_all_with_job_reference is what you had in mind - it's somewhat cumbersome to retrieve location from JobReference, unless I am missing on some simple trick (which is quite possible), so open to suggestions on that one

[lquerel]

@MikhailMS I will suggest to extract the location from the job_reference parameters as follow:

let location = job.location.map(|v| v.clone());

Location will be either None or Some(String) depending on the content of the JobReference.location field.

[MikhailMS]

@lquerel that's a weird one

If I try to get location the way you suggested

let location = job_reference.location.map(|v| v.clone());
let location = job_reference.location.as_ref().map(|v| v.clone()); #<< assumed this could be also an option
let location = job_reference.location.as_ref().and_then(|v| Some(v.clone())); #<< assumed this could be also an option

I'm getting move occurs because 'location' has type 'std::option::Option<std::string::String>' which does not implement the 'Copy' trait

Those happen if I try to extract location before JobReference gets consumed by job creation Once job gets created, JobReference is no longer accessible, so the only option left is to try and get location after insert() call, but it also complains about the move ...

I tried to do smth like

if let Some(ref job_ref) = job.job_reference.and_then(|r| Some(r)) {
                let mut page_token: Option<String> = None;
                loop {
                    if let Some(ref job_id) = job_ref.job_id {
                        if let Some(location) = &job_ref.location {
                            let qr = self
                                .get_query_results(

which almost worked, but we need Option<String> and the above return Option<&String> (and it won't without &job_reference)

Feel like I'm missing something simple here :/

UPD: got it sorted, but now

get_query_results(
        &self,
        project_id: &str,
        job_id: &str,
        parameters: &GetQueryResultsParameters, #<< instead of GetQueryResultsParameters
    )

[lquerel]

@MikhailMS I will publish a new version of this crate today. Thanks for your contribution.

[lquerel]

@MikhailMS I had to fix few issues here and there but it's now available on crates.io. Thanks