aoelvp94 commented on PR #73015: URL: https://github.com/apache/airflow/pull/73015#issuecomment-5854371583
Agreed — I'll drop the fallback. @ramitkataria's point decides it: it fires exactly when S3 is unhappy and turns one request into ~2N against the same prefix, from every staging pod. The trigger and the aggravating condition are the same event, so docs can't fix it. Both earlier findings in this PR were in that same path — dropping it removes the guard, the LIST probe and two doc caveats with it. I'd go further than just skipping it and make `archive_key` and `prefix` mutually exclusive. No second source, no drift, and one artifact to publish instead of two kept in sync. On the comparison: they optimize different axes. `get_object` drops the per-object HEAD (change detection already uses `size`/`last_modified` from ListObjects), halving requests — 802 → ~402 for 400 objects in my repro. Concurrency then gets wall-clock close to the archive. But neither takes request count below ~N, whereas the archive is 1 GET + 1 HEAD, and HEAD-only on an unchanged ETag. +1 to @vincbeck on both, as separate PRs: the archive is opt-in and inert when unset, whereas `sync_to_local_dir` affects every current user — and `providers/google` has a byte-identical copy of the same loop, so that one should probably cover both providers. Happy to take the sync work as a follow-up, or leave it to whoever wants it. Pushing the no-fallback version here meanwhile. -- 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]
