GCSToGCSOperator delimiter bug

[yukijam]

Apache Airflow version

2.5.1

What happened

Using GCSToGCSOperator with delimiter = '.json' would also consider the incorrect files
for example: I got a file named xxxx.jsonl__

What you think should happen instead

it should exclude the files not ending with .json

How to reproduce

No response

Operating System

gcp airflow composer

Versions of Apache Airflow Providers

No response

Deployment

Composer

Deployment details

No response

Anything else

No response

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[boring-cyborg]

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

[potiuk]

assigned you @yukijam if you want to fix it

[Taragolis]

Due to name of argument as delimiter, description and code of Operator and google-cloud-storage I thought it work correctly. However have no idea who actually need this functionality.

I thought better implement new argument with proper implementation and with name suffix

[shahar1]

@eladkal Please assign me

[potiuk]

Done

[shahar1]

Due to name of argument as delimiter, description and code of Operator and google-cloud-storage I thought it work correctly. However have no idea who actually need this functionality.

I thought better implement new argument with proper implementation and with name suffix

Based on [1] and [2], it seems that treating delimiter as suffix is a misuse, and filtering according to suffixes shouldn't be done as part of the API call to GCS but as separate logic within the hook (i.e., simple list filtering).
Therefore, I suggest deprecating the delimiter parameter (as it should almost always be "/") and adding a suffix parameter instead for use as a filter, as follows:
If delimiter is set and suffix is not already provided, warn that delimiter is deprecated and assign self.suffix its value, while self.delimiter will be set to a constant "/". If suffix is provided, it will behave as expected regardless of the value of delimiter.
The deprecation will apply only to operators that misuse the parameter, and not to the list() method of the GCS hook - as some operators might use it appropriately (gcs_to_gdrive, gcs_to_sftp).

Edit:

1. Ignore the previous suggestion for adding the suffix parameter.
2. Almost all operators misuse the delimiter param; a more deep change should be done.

I'll add details in a separate message.

[eladkal]

filtering according to suffixes shouldn't be done as part of the API call to GCS but as separate logic within the hook

I have ambivalent feelings about hooks doing more than the API/service allows.
If GCS decided to offer prefix & delimiter I'm not sure if we should create a additional functionally?

I think it would be easier to discuss over a PR where we can also see the test cases and get a sense of how much further customizations we do from Airflow side.