aoelvp94 commented on PR #73015: URL: https://github.com/apache/airflow/pull/73015#issuecomment-6417427708
> Thanks for the PR, I like the idea and I think it's not just for optimizing the cost and overhead. It might also improve the deployment side for the non-Python SDK deployments. > > Before we going further to adapt this pattern for all the object store based DagBundle. Would it be better to introduce a new class, e.g. `S3ArchiveDagBundle` instead of having on the existing `S3DagBundle`? > > From my perspective, the refreshing scope is completely different (objects under prefix map to dir vs single object map to dir) so having a independent class _might_ be more clear. > > Additionally, we _could_ even support versioning for the `S3ArchiveDagBundle`. IIUC, we're able to track the versioning id of that object from S3 as reference in Airflow meta database. > > cc @uranusjr who involved with non-Python SDK, @ephraimbuddy who experimented PoC for supporting versioning for object store based DagBundle. Thanks, this is the right question to settle before the same option lands on GCS and Azure. I'd keep `archive_key` on `S3DagBundle`. The two refresh scopes are different, which is why `archive_key` and `prefix` are mutually exclusive and rejected together at construction. With `archive_key` unset, the class behaves as it does today, so existing bundle config keeps the same classpath. A separate `S3ArchiveDagBundle` would repeat the hook, the lock, and the local path for a mode that is off unless configured. Versioning is the part I would not fold into this PR. An archive object has a version id, and a prefix does not, so `supports_versioning` on an archive class is a real follow-up. Both bundles still report `supports_versioning = False`. If we add that, a dedicated class is a better home, and I'm happy to split then. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
