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

Pass through more parameters to AzureBlobFileSystem to mirror handlin… #38747

Conversation

tomrutter
Copy link
Contributor

Pass a few extra parameters through from connection extras to adlfs.AzureBlobFileSystem to mirror handling of connection in WasbHook:

  • managed_identity_client_id
  • workload_identity_tenant_id
  • anon

closes: #38746


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@tomrutter tomrutter force-pushed the microsoft_azure_blob_storage_allow_managed_identity_objectstore branch from 1c695ab to 9e07242 Compare April 4, 2024 12:22
@tomrutter tomrutter force-pushed the microsoft_azure_blob_storage_allow_managed_identity_objectstore branch from 9e07242 to 7379f26 Compare April 4, 2024 12:26
@potiuk
Copy link
Member

potiuk commented Apr 7, 2024

unit tests?

@tomrutter
Copy link
Contributor Author

I've updated the code and added some unit tests. I've also updated the handling a little to handle more than just the specific use case I need right now. The intention is to bring the way that providers.microsoft.azure.fs.adls.get_fs handles a connection object to be closer to that of WasbHook.

@tomrutter tomrutter force-pushed the microsoft_azure_blob_storage_allow_managed_identity_objectstore branch from dfa9482 to 998f8aa Compare April 11, 2024 18:52
@tomrutter
Copy link
Contributor Author

Checked tests and linting pass, and rebased to latest main.

@potiuk
Copy link
Member

potiuk commented Apr 11, 2024

Tests failing, I am afraid.

…and add unit tests.

fix tests that were missed by keeping path of the account_url unchanged.
@tomrutter tomrutter force-pushed the microsoft_azure_blob_storage_allow_managed_identity_objectstore branch from 998f8aa to 8bf3cb7 Compare April 12, 2024 11:43
@tomrutter
Copy link
Contributor Author

Apologies, I realised too late that I missed the "db tests". Should all be ok now?

@potiuk potiuk merged commit db8471c into apache:main Apr 12, 2024
42 checks passed
@tomrutter
Copy link
Contributor Author

Thanks!

utkarsharma2 pushed a commit to astronomer/airflow that referenced this pull request Apr 22, 2024
…and add unit tests. (apache#38747)

fix tests that were missed by keeping path of the account_url unchanged.

Co-authored-by: Tom Rutter <tom.rutter@zurich.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Microsoft Azure blob storage doesn't allow managed identity connection in ObjectStore (but does in hook)
2 participants