Airflow file sensor by prefix for azure data lake storage #22315
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
|
|
Can you please add unit tests for it ? (see other unit tests for azure as an example). Also an example dag and ideally howto guide would be great. |
josh-fell
left a comment
josh-fell
left a comment
There was a problem hiding this comment.
+1 For adding unit tests, example DAG, and how-to guide would be great. All don't necessarily need to be part of this PR.
The airflow.contrib path is deprecated so we shouldn't add new modules there. This module should be part of airflow.providers.microsoft.azure.sensors.
I would recommend running static code checks if you can as well. This will make the CI run smoother when approved and decrease any review back-and-forth, fixes, etc.
| from airflow.contrib.hooks.azure_data_lake_hook import AzureDataLakeHook | ||
| from airflow.sensors.base_sensor_operator import BaseSensorOperator | ||
| from airflow.utils.decorators import apply_defaults |
There was a problem hiding this comment.
| from airflow.contrib.hooks.azure_data_lake_hook import AzureDataLakeHook | |
| from airflow.sensors.base_sensor_operator import BaseSensorOperator | |
| from airflow.utils.decorators import apply_defaults | |
| from airflow.providers.microsoft.azure.hooks.data_lake import AzureDataLakeHook | |
| from airflow.sensors.base import BaseSensorOperator |
The import paths for AzureDataLakeHook and BaseSensorOperator are deprecated. Also using apply_defaults is not necessary as of Airflow 2.
|
|
||
| """ | ||
|
|
||
| @apply_defaults |
There was a problem hiding this comment.
| @apply_defaults |
No longer needed as of Airflow 2.
| @apply_defaults | ||
| def __init__( | ||
| self, | ||
| DataLake_path, |
There was a problem hiding this comment.
We typically use snake case for variables, args, etc.
| DataLake_path, | ||
| prefix, | ||
| azure_data_lake_conn_id="azure_data_lake_default", | ||
| check_options=None, |
There was a problem hiding this comment.
This doesn't look used in the sensor. Should it be removed?
| DataLake_path : directory of the file | ||
|
|
||
| prefix : file name |
There was a problem hiding this comment.
Can you update the docstring for the parameters to match the Sphinx style used. The azure_data_lake_conn_id above is a good example or other examples in operators and sensors.
| Interacts with Azure Data Lake: | ||
|
|
||
| Client ID and client secret should be in user and password parameters. | ||
| Tenant and account name should be extra field as | ||
| {"tenant": "<TENANT>", "account_name": "ACCOUNT_NAME"}. |
There was a problem hiding this comment.
This description doesn't quite describe the purpose of the sensor. Would you mind updating and elaborating here?
| return len(hook.list(self.DataLake_path + "/" + self.prefix)) > 0 | ||
|
|
||
|
|
||
| # return TRUE => 1 ou more file detected | ||
| # return FALSE => No file detected |
There was a problem hiding this comment.
| return len(hook.list(self.DataLake_path + "/" + self.prefix)) > 0 | |
| # return TRUE => 1 ou more file detected | |
| # return FALSE => No file detected | |
| num_files = len(hook.list(self.DataLake_path + "/" + self.prefix)) | |
| return num_files > 0 |
Small refactor for readability. The comments are better suited for the sensor's description in the docstring since this is the fundamental operation of the sensor.
|
I am going to release a new version of providers in ~ week so if you want to get it included @Ash-ZAMAN - good time to address the comments. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
github-actions
bot
added
the
stale
4 participants
Hi,
I noticed there was no airflow file sensor suitable for Azure Data Lake Storage. Hence, I created one. That could be realy useful for people who store data in Data Lake.
My sensor is adaptable with the prefix of the file's name and any directory path
hope this contribution will help someone.
Regards, Ashraf ZAMAN
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.