Skip to content
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

Wasb connection to Azure Fileshareservice Hook not working #16254

Closed
cvsekhar opened this issue Jun 3, 2021 · 8 comments · Fixed by #16388
Closed

Wasb connection to Azure Fileshareservice Hook not working #16254

cvsekhar opened this issue Jun 3, 2021 · 8 comments · Fixed by #16388
Labels
area:providers kind:bug This is a clearly a bug

Comments

@cvsekhar
Copy link

cvsekhar commented Jun 3, 2021

Apache Airflow version:
Version: v2.1.0
Git Version: .release:2.1.0+304e174674ff6921cb7ed79c0158949b50eff8fe

What happened:
INFO - init() got an unexpected keyword argument 'extra__wasb__connection_string'

What you expected to happen:
We have used the extra field and populated the SAS token according to the documents on how to use wasb connection for fileshare and we should be able to login in using the SAS token. This has been working in 2.0.0

airflow_bug

@cvsekhar cvsekhar added the kind:bug This is a clearly a bug label Jun 3, 2021
@boring-cyborg
Copy link

boring-cyborg bot commented Jun 3, 2021

Thanks for opening your first issue here! Be sure to follow the issue template!

@cvsekhar
Copy link
Author

cvsekhar commented Jun 3, 2021

When we try to enter key-value pairs in JSON format for file service in the wasb connection those key-value pairs are not being stored and get replaced with {"extra__wasb__connection_string": "", "extra__wasb__sas_token": "", "extra__wasb__shared_access_key": "", "extra__wasb__tenant_id": ""}

@potiuk
Copy link
Member

potiuk commented Jun 5, 2021

When using the UI, you should not configure extras but you can now place the values in the right fields - instead of manually adding "extras" via JSON dict:

You have those fields (as you can see in your screenshot):

  • SAS token
  • Tenant ID
  • Shared access key

Which should be used for that purpose.

This has been fixed in The Microsoft Azure Provider 2.0.0 (as a breaking change):

https://airflow.apache.org/docs/apache-airflow-providers-microsoft-azure/stable/index.html#id1

The detailed commit:
1a85ba9

@potiuk
Copy link
Member

potiuk commented Jun 5, 2021

Closing for now unless it does not solve your problem (please let us know if it did/did not).

@potiuk potiuk closed this as completed Jun 5, 2021
@cvsekhar
Copy link
Author

cvsekhar commented Jun 7, 2021

This has not addressed the issue, I was using those fields and getting an error when I try to use this hook.

filesharehook

@cvsekhar
Copy link
Author

cvsekhar commented Jun 7, 2021

I see from the commit list, this specific class has not be addressed and its giving an error.
commit_Screenshot

@potiuk potiuk reopened this Jun 7, 2021
@potiuk
Copy link
Member

potiuk commented Jun 7, 2021

OK. I see the problem @cvsekhar . It seems that the AzureFileServerHook was wrongly defined and the way extras were used there was wrong. Passing extras this way is not how airlfow works with connection extras - all extras are always prefixed with extra__conn__ and your extras (and they way how the FileServerHook works).

The AzureFileServerHook has been missing in the change and it does not rally work the same way - on one hand you have no dedicated hook, and on the other hand - the other "Azure" related hooks have some extras defined which are not properly configured.

There are few ways you can mitigate the problem:

  1. downgrade apache-airflow-providers-microsoft-azure to 1.3.0 - that is the most "certain" fix for you I think.

  2. You can try to use the "Azure" connection instead in 2.0.0. Though it will likely fail with "extra__azure__tenantId" missing - looking at the code.

  3. As a quick workaround, you should be able to use any "generic" connection. For example you can use HTTP connection and fill in the extras the way you used it. It should work just fine (though it is an abuse of how Airlfow connections work). This is probably the "fastest" workaround if it works for you

Let me know if those work.

In the meantime I might implement a quick fix - I am just about to release a new version of all providers but I think I can quickly fix this one in that upcoming version. I will ask you to test the RC version then OK ?

@cvsekhar
Copy link
Author

cvsekhar commented Jun 7, 2021

@potiuk, thank you Sure will test the RC

potiuk added a commit to potiuk/airflow that referenced this issue Jun 11, 2021
The Azure File Share connection has not been creted in apache#15159 and it
caused an unexpected side effect as the default Azure Connection
passed service_options dictionary to FileService
with key that was unexpected.

This change fixes two things:

1) adds AzureFileShare connection that has separate conn_type
   and handles the extra_options specific for FileService Hook
   available in the Airflow UI.

2) handles the "deprecated" way of passing keys without UI prefix
   but raises a deprecation warning when such key is passed or
   when the Wasb connection is used with an empty extras rather
   than Azure File Share.

Fixes apache#16254
potiuk added a commit that referenced this issue Jun 11, 2021
* Fixes AzureFileShare connection extras

The Azure File Share connection has not been creted in #15159 and it
caused an unexpected side effect as the default Azure Connection
passed service_options dictionary to FileService
with key that was unexpected.

This change fixes two things:

1) adds AzureFileShare connection that has separate conn_type
   and handles the extra_options specific for FileService Hook
   available in the Airflow UI.

2) handles the "deprecated" way of passing keys without UI prefix
   but raises a deprecation warning when such key is passed or
   when the Wasb connection is used with an empty extras rather
   than Azure File Share.

Fixes #16254

* fixup! Fixes AzureFileShare connection extras

* fixup! fixup! Fixes AzureFileShare connection extras
potiuk added a commit to potiuk/airflow that referenced this issue Jun 22, 2021
* Fixes AzureFileShare connection extras

The Azure File Share connection has not been creted in apache#15159 and it
caused an unexpected side effect as the default Azure Connection
passed service_options dictionary to FileService
with key that was unexpected.

This change fixes two things:

1) adds AzureFileShare connection that has separate conn_type
   and handles the extra_options specific for FileService Hook
   available in the Airflow UI.

2) handles the "deprecated" way of passing keys without UI prefix
   but raises a deprecation warning when such key is passed or
   when the Wasb connection is used with an empty extras rather
   than Azure File Share.

Fixes apache#16254

* fixup! Fixes AzureFileShare connection extras

* fixup! fixup! Fixes AzureFileShare connection extras

(cherry picked from commit 0c80a7d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers kind:bug This is a clearly a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants