@shahar1 due to a small mistake i accidentally pushed the all commits and got auto assigned reviewers. Sorry for that.

And Also Shahar this we are using everywhere in the operator instead of that we can use instance.exists() in one line in hooks right??

        instance = hook.get_instance(project_id=self.project_id, instance_id=self.instance_id)
        if not instance:
            raise AirflowException(f"Dependency: instance '{self.instance_id}' does not exist.")

instance = hook.get_instance(project_id=self.project_id, instance_id=self.instance_id)

You may encapsulate it in the hook where applicable

Okay Sure

I have updated the code according to your comments. Please have a view.

There are many open GCP issues(some are very older that i don't know whether they are relevant or not). If you don't mind can you assign me any of those issues. I would be happy to work on those.

Sounds good, please tag me in a couple of issues and I'll assign you if they are still relevant.

Sure I will tag you

@shahar1 All Checks have passed!

@shahar1 All Checks have passed!

Great! I'll give Google's team a couple of more days to comment, and then I'll merge.

Shahar I have one suggestion. In Bigtable Hooks currently we are having this get_cluster_states_for_table (it fetches the clusters associated with a particular table) but which is currently not used by any Operator.

For BigtableCreateTableOperator, since clusters are created by default while creating a table, would it make sense to reuse this hook method and log the number (and possibly state) of clusters associated with the table after creation?

Whats your view in this??

get_cluster_states_for_table

Shahar I have one suggestion. In Bigtable Hooks currently we are having this get_cluster_states_for_table (it fetches the clusters associated with a particular table) but which is currently not used by any Operator.

For BigtableCreateTableOperator, since clusters are created by default while creating a table, would it make sense to reuse this hook method and log the number (and possibly state) of clusters associated with the table after creation?

Whats your view in this??

It's being used in the BigtableTableReplicationCompletedSensor sensor.
I don't think it makes sense to be used in BigtableCreateTableOperator, as clusters are not created by default (you run hook.get_instance() to get them, they are not auto-provisioned).

Following a recent incident - I want also to ensure that related system tests pass before merging this one.
@KamranImaaz - if you're able to run them, please go ahead and attach screenshots. Otherwise, we'll have to wait for either Google team or anyone else to run them.

CC: @VladaZakharova @KamranImaaz

Following a recent incident - I want also to ensure that related system tests pass before merging this one. @KamranImaaz - if you're able to run them, please go ahead and attach screenshots. Otherwise, we'll have to wait for either Google team or anyone else to run them.

CC: @VladaZakharova @KamranImaaz

Hi @shahar1 while I currently do not have the access to run the system tests. I will try to get the access and run them. In the meanwhile google team can run them

LGTM

Just to make it clear - are you ok with merging without running the system tests?
Personally I don't find anything that could be broken in the system tests due to the suggested changes, but it was @VladaZakharova 's request - so I want to ensure that we're all aligned.
If you are able to run it on your side before merging it would be the best, so we could avoid unnecessary reverts later on.
I'm also still waiting to chat with your team regarding the integration of Google provider system tests into the CI.
Thanks!

Ran it myself eventually, we're good to go:

@KamranImaaz - Great job!

Thanks