[GitHub] potiuk edited a comment on issue #4458: [AIRFLOW-3648] Default to connection project id in gcp cloud sql.

[GitBox <gi...@apache.org>]

potiuk edited a comment on issue #4458: [AIRFLOW-3648] Default to connection project id in gcp cloud sql.
URL: https://github.com/apache/airflow/pull/4458#issuecomment-452422541
 
 
   Hey @jmcarp. I actually promised to @kaxil that I will do it this week, and I have already done it yesterday :).  Including system testing with real GCP project for all operators we developed for GCP. 
   
   I have a big internal PR opened with 15 separate commits where this is fixed https://github.com/PolideaInternal/airflow/pull/29.  I have started with @kaxil discusion on the best way how it can be split so it can be reviewed fast and quickly and that we can also merge the changes which allow us to run automatically System Tests with real GCP project for all GCP-related operators. 
   
   The cloud SQL one is implemented as separate commit https://github.com/PolideaInternal/airflow/pull/29/commits/097f024e1960ec1ab47e980fef2aba5bdafe3e02 in the (so far big) PR. Please do not comment it just yet, it's very likely it will be split to at least 3 or 4 PRs or maybe more. It includes unit tests changes, it also changes the example dags (allowing to run automated system tests) and shows examples of how you can create the operators in the official documention - showing examples with and without project_id.
   
   My change is done slightly differently: rather than adding project_id=None to hooks I've only changed the operators interface and added a decorator in the hooks that adds project_id from the hook automatically if it is set to None. This is a bit nicer I think because the logic to set the project_id is done in one place only (in the decorator) common for all the operators and you do not have to remember to handle it in all hooks separately. You just need to document that project_id can be none. I could also think about reverting the parameters in hook and having project_id = None as you did, but I thought it's not worth it (but I might reconsider). We have the full automated suite of system tests and automated cloud build that performs the tests on push to our repo (https://storage.googleapis.com/polidea-airflow-builds/56d1ea7c-20c9-4619-8d23-7d0d1c597059/index.html you can see an example success after running System Tests for all GCP operators). And here you can see the documentation generated automatically during that verification build: https://storage.googleapis.com/polidea-airflow-builds/56d1ea7c-20c9-4619-8d23-7d0d1c597059/docs/howto/operator.html#cloudsqlinstancedatabasecreateoperator where you can see that we have now examples with and without project_id.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services