-
Notifications
You must be signed in to change notification settings - Fork 304
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
The magics API do not support setting client options #160
Comments
@boazsade Thanks for the suggestion and the patch! I'll just check if Product has any objections (cc: @shollyman), and if not, we'll add this. P.S.: If confirmed, would you mind opening an actual PR with added tests, or are you fine with us taking the patch as a base and create a PR ourselves? |
You can take this pach, this is something that we really need to be part of the code base, thanks for the reply |
@plamut is there any progress with this issue? I was unable to create pull request, what about applying the patch? was that approved? thanks |
@boazsade Not at this point, unfortunately, but I can re-check at this week's meeting. Lately one of the primary things in focus are preparations to dropping Python 2 support and moving them to the new code generator (for those who have some auto-generated parts). |
Hi @plamut. I wanted to check in and see if there's an update regarding including this patch in an upcoming release. One of our customers is in the process of migrating their data environment to BigQuery and this has become a priority. Thanks in advance. |
@yoavcloud Hi, I'm working on a PR to add your patch. Should be ready for review in a couple of days. |
Thank you so much @cguardia! |
@yoavcloud Well, I've been looking at this and have a couple of questions. While |
The patch file was generated by me (@boazsade), this patch was generate after we tested this to work. This was the reason we wanted it - after verify it. What we really need is the option to set the end point value (so that the traffic can pass not directly to google but through other authenticated host. As for the other parameters (like quota_project_id) I didn't test that, the reason I wrote it the way I did was to keep it with the same as the internal calls, but this can be dropped - again we really need the option to set the api_endpoint, other parameters as less important for us. |
We don't necessarily need command-line options for all of these (though it wouldn't hurt to add). The most important thing is to make sure it is settable from |
@boazsade sorry about my confusion with the patch's authorship. Setting the client options directly on the context worked, but the command line options did not, so I changed the patch to add an |
good to here it, looking forward for this |
Thank you everyone! |
Environment details
Steps to reproduce
This is a missing option that do exists in google bigquery client - "https://googleapis.dev/python/bigquery/latest/generated/google.cloud.bigquery.client.Client.html#google.cloud.bigquery.client.Client"
But is missing from "https://github.com/googleapis/python-bigquery/blob/master/google/cloud/bigquery/magics.py"
Code example
Note the the client_option is missing in creating the client in the code above
The patch to apply to fix this flow:
The text was updated successfully, but these errors were encountered: