You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "jroachgolf84 (via GitHub)" <gi...@apache.org> on 2025/08/29 13:53:10 UTC
[PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
jroachgolf84 opened a new pull request, #55070:
URL: https://github.com/apache/airflow/pull/55070
This PR aims to reduce the memory footprint for the `S3KeySensor`. This was done by altering the `get_file_metadata` method in the S3Hook to `yield` records in the paginated response, rather than loading them into a single list. The return type for the `get_file_metadata` method is not an `Iterator`. An assertion was added to validate this, and all appropriate tests were updated.
closes: #55039
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2310283140
##########
providers/amazon/src/airflow/providers/amazon/aws/sensors/s3.py:
##########
@@ -132,21 +144,23 @@ def _check_key(self, key, context: Context):
for f in key_matches:
metadata = {}
if "*" in self.metadata_keys:
- metadata = self.hook.head_object(f["Key"], bucket_name)
+ metadata = self.hook.head_object(f["Key"], bucket_name) # type: ignore[index]
else:
- for key in self.metadata_keys:
+ for mk in self.metadata_keys:
try:
- metadata[key] = f[key]
+ metadata[mk] = f[mk] # type: ignore[index]
except KeyError:
# supplied key might be from head_object response
- self.log.info("Key %s not found in response, performing head_object", key)
- metadata[key] = self.hook.head_object(f["Key"], bucket_name).get(key, None)
+ self.log.info("Key %s not found in response, performing head_object", mk)
+ metadata[mk] = self.hook.head_object(f["Key"], bucket_name).get(mk, None) # type: ignore[index]
files.append(metadata)
+
Review Comment:
no prob
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2310258693
##########
providers/amazon/src/airflow/providers/amazon/aws/sensors/s3.py:
##########
@@ -132,21 +144,23 @@ def _check_key(self, key, context: Context):
for f in key_matches:
metadata = {}
if "*" in self.metadata_keys:
- metadata = self.hook.head_object(f["Key"], bucket_name)
+ metadata = self.hook.head_object(f["Key"], bucket_name) # type: ignore[index]
else:
- for key in self.metadata_keys:
+ for mk in self.metadata_keys:
try:
- metadata[key] = f[key]
+ metadata[mk] = f[mk] # type: ignore[index]
except KeyError:
# supplied key might be from head_object response
- self.log.info("Key %s not found in response, performing head_object", key)
- metadata[key] = self.hook.head_object(f["Key"], bucket_name).get(key, None)
+ self.log.info("Key %s not found in response, performing head_object", mk)
+ metadata[mk] = self.hook.head_object(f["Key"], bucket_name).get(mk, None) # type: ignore[index]
files.append(metadata)
+
Review Comment:
The previous developer overwrote the `key` field they defined at the method-level within the `for` loop. This was more of a "maintenance" fix.
The `# type: ignore[index]` was needed to pass pre-commit.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2310282469
##########
providers/amazon/src/airflow/providers/amazon/aws/sensors/s3.py:
##########
@@ -132,21 +144,23 @@ def _check_key(self, key, context: Context):
for f in key_matches:
metadata = {}
if "*" in self.metadata_keys:
- metadata = self.hook.head_object(f["Key"], bucket_name)
+ metadata = self.hook.head_object(f["Key"], bucket_name) # type: ignore[index]
else:
- for key in self.metadata_keys:
+ for mk in self.metadata_keys:
try:
- metadata[key] = f[key]
+ metadata[mk] = f[mk] # type: ignore[index]
except KeyError:
# supplied key might be from head_object response
- self.log.info("Key %s not found in response, performing head_object", key)
- metadata[key] = self.hook.head_object(f["Key"], bucket_name).get(key, None)
+ self.log.info("Key %s not found in response, performing head_object", mk)
+ metadata[mk] = self.hook.head_object(f["Key"], bucket_name).get(mk, None) # type: ignore[index]
files.append(metadata)
+
elif self.use_regex:
- keys = self.hook.get_file_metadata("", bucket_name)
- key_matches = [k for k in keys if re.match(pattern=key, string=k["Key"])]
- if not key_matches:
- return False
+ for k in self.hook.get_file_metadata("", bucket_name):
Review Comment:
it is, just i'm curious about it -- disregard
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2310256180
##########
providers/amazon/src/airflow/providers/amazon/aws/sensors/s3.py:
##########
@@ -132,21 +144,23 @@ def _check_key(self, key, context: Context):
for f in key_matches:
metadata = {}
if "*" in self.metadata_keys:
- metadata = self.hook.head_object(f["Key"], bucket_name)
+ metadata = self.hook.head_object(f["Key"], bucket_name) # type: ignore[index]
else:
- for key in self.metadata_keys:
+ for mk in self.metadata_keys:
try:
- metadata[key] = f[key]
+ metadata[mk] = f[mk] # type: ignore[index]
except KeyError:
# supplied key might be from head_object response
- self.log.info("Key %s not found in response, performing head_object", key)
- metadata[key] = self.hook.head_object(f["Key"], bucket_name).get(key, None)
+ self.log.info("Key %s not found in response, performing head_object", mk)
+ metadata[mk] = self.hook.head_object(f["Key"], bucket_name).get(mk, None) # type: ignore[index]
files.append(metadata)
+
elif self.use_regex:
- keys = self.hook.get_file_metadata("", bucket_name)
- key_matches = [k for k in keys if re.match(pattern=key, string=k["Key"])]
- if not key_matches:
- return False
+ for k in self.hook.get_file_metadata("", bucket_name):
Review Comment:
why doesn't this use the prefix -- odd
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2310250629
##########
providers/amazon/src/airflow/providers/amazon/aws/sensors/s3.py:
##########
@@ -132,21 +144,23 @@ def _check_key(self, key, context: Context):
for f in key_matches:
metadata = {}
if "*" in self.metadata_keys:
- metadata = self.hook.head_object(f["Key"], bucket_name)
+ metadata = self.hook.head_object(f["Key"], bucket_name) # type: ignore[index]
else:
- for key in self.metadata_keys:
+ for mk in self.metadata_keys:
try:
- metadata[key] = f[key]
+ metadata[mk] = f[mk] # type: ignore[index]
except KeyError:
# supplied key might be from head_object response
- self.log.info("Key %s not found in response, performing head_object", key)
- metadata[key] = self.hook.head_object(f["Key"], bucket_name).get(key, None)
+ self.log.info("Key %s not found in response, performing head_object", mk)
+ metadata[mk] = self.hook.head_object(f["Key"], bucket_name).get(mk, None) # type: ignore[index]
files.append(metadata)
+
Review Comment:
these all seem unrelated / unnecessary?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2310262288
##########
providers/amazon/src/airflow/providers/amazon/aws/sensors/s3.py:
##########
@@ -132,21 +144,23 @@ def _check_key(self, key, context: Context):
for f in key_matches:
metadata = {}
if "*" in self.metadata_keys:
- metadata = self.hook.head_object(f["Key"], bucket_name)
+ metadata = self.hook.head_object(f["Key"], bucket_name) # type: ignore[index]
else:
- for key in self.metadata_keys:
+ for mk in self.metadata_keys:
try:
- metadata[key] = f[key]
+ metadata[mk] = f[mk] # type: ignore[index]
except KeyError:
# supplied key might be from head_object response
- self.log.info("Key %s not found in response, performing head_object", key)
- metadata[key] = self.hook.head_object(f["Key"], bucket_name).get(key, None)
+ self.log.info("Key %s not found in response, performing head_object", mk)
+ metadata[mk] = self.hook.head_object(f["Key"], bucket_name).get(mk, None) # type: ignore[index]
files.append(metadata)
+
elif self.use_regex:
- keys = self.hook.get_file_metadata("", bucket_name)
- key_matches = [k for k in keys if re.match(pattern=key, string=k["Key"])]
- if not key_matches:
- return False
+ for k in self.hook.get_file_metadata("", bucket_name):
Review Comment:
This change felt out-of-scope, but happy to play around a bit with that?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3245585583
cc: @eladkal
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3249028980
I think that's feasible - how would we determine whether to return a `list` or an `Iterator`?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb merged PR #55070:
URL: https://github.com/apache/airflow/pull/55070
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3249864758
In the current implementation we would always return an iterator but having `list | Iterator` as return type annotation would keep it backward compatible
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2310241361
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -957,11 +957,10 @@ def get_file_metadata(
params["RequestPayer"] = "requester"
response = paginator.paginate(**params)
- files = []
+ # Rather than returning a list and loading all keys into memory, we'll iterate over the results
Review Comment:
Updated, thanks.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2359585799
##########
providers/amazon/src/airflow/providers/amazon/aws/sensors/s3.py:
##########
@@ -122,8 +122,20 @@ def _check_key(self, key, context: Context):
"""
if self.wildcard_match:
prefix = re.split(r"[\[*?]", key, 1)[0]
- keys = self.hook.get_file_metadata(prefix, bucket_name)
- key_matches = [k for k in keys if fnmatch.fnmatch(k["Key"], key)]
+
+ key_matches: list[str] = []
+
+ # Is check_fn is None, then we can return True without having to iterate through each value in
+ # yielded by iter_file_metadata. Otherwise, we'll check for a match, and add all matches to the
+ # key_matches list
+ for k in self.hook.iter_file_metadata(prefix, bucket_name):
+ if fnmatch.fnmatch(k["Key"], key):
+ if self.check_fn is not None:
+ key_matches.append(k)
Review Comment:
Agreed - I'm inclined to change that functionality outside of things PR; it's used quite a bit elsewhere in the AWS Provider.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3237191496
mentioned it on slack but just adding here
should check with @eladkal re backcompat issues. changing `get_file_metadata` to iterator likely means major release. and given that maybe we should update the whole check key funciton logic so that it _never_ has to hold everything in memory, i.e. by making it so that check_fn takes a key and not a list
but thinking about that..... the problem is it's somewhat ambiguous what the behavior should be
for best performance, you would want check_fn to return on first "pass"
but, the behavior that would be most similar to current, would be to return all the files for which check_fn evaluates to true
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3255805481
> Actually sorry but I realized I provided wrong instructions. This does not solve back compat issues ... What we should do instead is deprecate `get_file_metadata` and create a new one using `Iterator` and use this one in the sensor. That way we do not need to create a major release because of this.
I guess just wondering what is wrong with making a major release? How do we determine when to proceed with one?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3254837378
@vincbeck, I've updated the PR accordingly!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2358010858
##########
providers/amazon/src/airflow/providers/amazon/aws/sensors/s3.py:
##########
@@ -122,8 +122,20 @@ def _check_key(self, key, context: Context):
"""
if self.wildcard_match:
prefix = re.split(r"[\[*?]", key, 1)[0]
- keys = self.hook.get_file_metadata(prefix, bucket_name)
- key_matches = [k for k in keys if fnmatch.fnmatch(k["Key"], key)]
+
+ key_matches: list[str] = []
+
+ # Is check_fn is None, then we can return True without having to iterate through each value in
+ # yielded by iter_file_metadata. Otherwise, we'll check for a match, and add all matches to the
+ # key_matches list
+ for k in self.hook.iter_file_metadata(prefix, bucket_name):
+ if fnmatch.fnmatch(k["Key"], key):
+ if self.check_fn is not None:
+ key_matches.append(k)
Review Comment:
Oh. check_fn takes a single list. That seems like a poor design decision :(
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3237121304
cc: @dstandish
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2358004652
##########
providers/amazon/src/airflow/providers/amazon/aws/sensors/s3.py:
##########
@@ -122,8 +122,20 @@ def _check_key(self, key, context: Context):
"""
if self.wildcard_match:
prefix = re.split(r"[\[*?]", key, 1)[0]
- keys = self.hook.get_file_metadata(prefix, bucket_name)
- key_matches = [k for k in keys if fnmatch.fnmatch(k["Key"], key)]
+
+ key_matches: list[str] = []
+
+ # Is check_fn is None, then we can return True without having to iterate through each value in
+ # yielded by iter_file_metadata. Otherwise, we'll check for a match, and add all matches to the
+ # key_matches list
+ for k in self.hook.iter_file_metadata(prefix, bucket_name):
+ if fnmatch.fnmatch(k["Key"], key):
+ if self.check_fn is not None:
+ key_matches.append(k)
+ # This will only wait for a single match, and will immediately return
+ else:
+ return True
Review Comment:
Style "nit" for clarity -- generally try to keep code as un-indented as possible
```suggestion
if self.check_fn is None:
# This will only wait for a single match, and will immediately return
return True
key_matches.append(k)
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3246565120
To keep it backward compatible, could we make `get_file_metadata ` return `list | Iterator`? We would need to handle both cases in `providers/amazon/src/airflow/providers/amazon/aws/sensors/s3.py` which I do not really like but that's okay. What do you think?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3253859297
> In the current implementation we would always return an iterator but having `list | Iterator` as return type annotation would keep it backward compatible
Making the type annotation change now. Both cases are handled in `providers/amazon/src/airflow/providers/amazon/aws/sensors/s3.py`.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2325468659
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -931,7 +932,51 @@ def get_file_metadata(
max_items: int | None = None,
) -> list:
"""
- List metadata objects in a bucket under prefix.
+ [DEPRECATED] Retrieve metadata objects from a bucket under a prefix.
+
+ This method `get_file_metadata` is deprecated. Calling this method will result in all matching keys
+ being loaded into a single list, and can often result in out-of-memory exceptions. Instead, use
+ `yield_file_metadata`.
+ """ # noqa: D401
+ warnings.warn(
+ "This method `get_file_metadata` is deprecated. Calling this method will result in all matching "
+ "keys being loaded into a single list, and can often result in out-of-memory exceptions. "
+ "Instead, use `yield_file_metadata`.",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
+ )
+
+ config = {
+ "PageSize": page_size,
+ "MaxItems": max_items,
+ }
+
+ paginator = self.get_conn().get_paginator("list_objects_v2")
+ params = {
+ "Bucket": bucket_name,
+ "Prefix": prefix,
+ "PaginationConfig": config,
+ }
+ if self._requester_pays:
+ params["RequestPayer"] = "requester"
+ response = paginator.paginate(**params)
+
+ files = []
+ for page in response:
+ if "Contents" in page:
+ files += page["Contents"]
+ return files
+
+ @provide_bucket_name
+ def yield_file_metadata(
Review Comment:
I'm going to change this to `iter_file_metadata`.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2357400726
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -931,7 +932,51 @@ def get_file_metadata(
max_items: int | None = None,
) -> list:
"""
- List metadata objects in a bucket under prefix.
+ [DEPRECATED] Retrieve metadata objects from a bucket under a prefix.
+
Review Comment:
this is optional. Can we add
`.. deprecated:: <version>` instead of writing [DEPRECATED]
Above will show up in docs. similar to [this](https://airflow.apache.org/docs/apache-airflow-providers-amazon/stable/_api/airflow/providers/amazon/aws/hooks/glue/index.html#airflow.providers.amazon.aws.hooks.glue.GlueJobHook.list_jobs)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3308039990
@ashb should be all set here!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2324958378
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -931,7 +932,51 @@ def get_file_metadata(
max_items: int | None = None,
) -> list:
"""
- List metadata objects in a bucket under prefix.
+ [DEPRECATED] Retrieve metadata objects from a bucket under a prefix.
+
+ This method `get_file_metadata` is deprecated. Calling this method will result in all matching keys
+ being loaded into a single list, and can often result in out-of-memory exceptions. Instead, use
+ `yield_file_metadata`.
+ """ # noqa: D401
+ warnings.warn(
+ "This method `get_file_metadata` is deprecated. Calling this method will result in all matching "
+ "keys being loaded into a single list, and can often result in out-of-memory exceptions. "
+ "Instead, use `yield_file_metadata`.",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
+ )
+
+ config = {
+ "PageSize": page_size,
+ "MaxItems": max_items,
+ }
+
+ paginator = self.get_conn().get_paginator("list_objects_v2")
+ params = {
+ "Bucket": bucket_name,
+ "Prefix": prefix,
+ "PaginationConfig": config,
+ }
+ if self._requester_pays:
+ params["RequestPayer"] = "requester"
+ response = paginator.paginate(**params)
+
+ files = []
+ for page in response:
+ if "Contents" in page:
+ files += page["Contents"]
+ return files
+
+ @provide_bucket_name
+ def yield_file_metadata(
Review Comment:
@uranusjr - any thoughts here?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2323600312
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -931,7 +932,51 @@ def get_file_metadata(
max_items: int | None = None,
) -> list:
"""
- List metadata objects in a bucket under prefix.
+ [DEPRECATED] Retrieve metadata objects from a bucket under a prefix.
+
+ This method `get_file_metadata` is deprecated. Calling this method will result in all matching keys
+ being loaded into a single list, and can often result in out-of-memory exceptions. Instead, use
+ `yield_file_metadata`.
+ """ # noqa: D401
+ warnings.warn(
+ "This method `get_file_metadata` is deprecated. Calling this method will result in all matching "
+ "keys being loaded into a single list, and can often result in out-of-memory exceptions. "
+ "Instead, use `yield_file_metadata`.",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
+ )
+
+ config = {
+ "PageSize": page_size,
+ "MaxItems": max_items,
+ }
+
+ paginator = self.get_conn().get_paginator("list_objects_v2")
+ params = {
+ "Bucket": bucket_name,
+ "Prefix": prefix,
+ "PaginationConfig": config,
+ }
+ if self._requester_pays:
+ params["RequestPayer"] = "requester"
+ response = paginator.paginate(**params)
+
+ files = []
+ for page in response:
+ if "Contents" in page:
+ files += page["Contents"]
+ return files
+
+ @provide_bucket_name
+ def yield_file_metadata(
Review Comment:
probably `iter_file_metadata` would be better. maybe @uranusjr has a suggestion.
still not sure why not just do major release
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3254570958
> Got it - so the plan would be to:
>
> Deprecate get_file_metadata with a deprecation warning?
> Create a new get_file_metadata method that returns an Iterator, and use that in the Sensor.
Correct
> What is the best way to deprecate a method and create one with the same name?
To deprecate a method, please emit deprecation warning at the beginning of the method. Example below:
```
warnings.warn(
"The property `async_conn` is deprecated. Accessing it in an async context will cause the event loop to block. "
"Use the async method `get_async_conn` instead.",
AirflowProviderDeprecationWarning,
stacklevel=2,
)
```
> Or should I create a new method with a new name, and use this one in the Sensor?
Yes, you should create a new one with a new name
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3246502158
Lets consult first with @o-nikolas and @vincbeck.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2357991590
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -931,7 +932,51 @@ def get_file_metadata(
max_items: int | None = None,
) -> list:
"""
- List metadata objects in a bucket under prefix.
+ [DEPRECATED] Retrieve metadata objects from a bucket under a prefix.
+
+ This method `get_file_metadata` is deprecated. Calling this method will result in all matching keys
+ being loaded into a single list, and can often result in out-of-memory exceptions. Instead, use
+ `iter_file_metadata`.
+ """ # noqa: D401
+ warnings.warn(
+ "This method `get_file_metadata` is deprecated. Calling this method will result in all matching "
+ "keys being loaded into a single list, and can often result in out-of-memory exceptions. "
+ "Instead, use `iter_file_metadata`.",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
+ )
+
+ config = {
+ "PageSize": page_size,
+ "MaxItems": max_items,
+ }
+
+ paginator = self.get_conn().get_paginator("list_objects_v2")
+ params = {
+ "Bucket": bucket_name,
+ "Prefix": prefix,
+ "PaginationConfig": config,
+ }
+ if self._requester_pays:
+ params["RequestPayer"] = "requester"
+ response = paginator.paginate(**params)
+
+ files = []
+ for page in response:
+ if "Contents" in page:
+ files += page["Contents"]
+ return files
Review Comment:
Could this be
```python
return [self.iter_file_metadata(prefix, bucket_name, page_size, max_items)]
```
instead of a full fn?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2359622387
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -931,7 +932,51 @@ def get_file_metadata(
max_items: int | None = None,
) -> list:
"""
- List metadata objects in a bucket under prefix.
+ [DEPRECATED] Retrieve metadata objects from a bucket under a prefix.
+
+ This method `get_file_metadata` is deprecated. Calling this method will result in all matching keys
+ being loaded into a single list, and can often result in out-of-memory exceptions. Instead, use
+ `iter_file_metadata`.
+ """ # noqa: D401
+ warnings.warn(
+ "This method `get_file_metadata` is deprecated. Calling this method will result in all matching "
+ "keys being loaded into a single list, and can often result in out-of-memory exceptions. "
+ "Instead, use `iter_file_metadata`.",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
+ )
+
+ config = {
+ "PageSize": page_size,
+ "MaxItems": max_items,
+ }
+
+ paginator = self.get_conn().get_paginator("list_objects_v2")
+ params = {
+ "Bucket": bucket_name,
+ "Prefix": prefix,
+ "PaginationConfig": config,
+ }
+ if self._requester_pays:
+ params["RequestPayer"] = "requester"
+ response = paginator.paginate(**params)
+
+ files = []
+ for page in response:
+ if "Contents" in page:
+ files += page["Contents"]
+ return files
Review Comment:
Implemented the below:
```python
return list(self.iter_file_metadata(prefix, bucket_name, page_size, max_items))
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2359635154
##########
providers/amazon/src/airflow/providers/amazon/aws/sensors/s3.py:
##########
@@ -122,8 +122,20 @@ def _check_key(self, key, context: Context):
"""
if self.wildcard_match:
prefix = re.split(r"[\[*?]", key, 1)[0]
- keys = self.hook.get_file_metadata(prefix, bucket_name)
- key_matches = [k for k in keys if fnmatch.fnmatch(k["Key"], key)]
+
+ key_matches: list[str] = []
+
+ # Is check_fn is None, then we can return True without having to iterate through each value in
+ # yielded by iter_file_metadata. Otherwise, we'll check for a match, and add all matches to the
+ # key_matches list
+ for k in self.hook.iter_file_metadata(prefix, bucket_name):
+ if fnmatch.fnmatch(k["Key"], key):
+ if self.check_fn is not None:
+ key_matches.append(k)
+ # This will only wait for a single match, and will immediately return
+ else:
+ return True
Review Comment:
Thanks - this wasn't my prettiest code :)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3292132418
@dstandish, can you take a look at the updated PR?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2357400726
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -931,7 +932,51 @@ def get_file_metadata(
max_items: int | None = None,
) -> list:
"""
- List metadata objects in a bucket under prefix.
+ [DEPRECATED] Retrieve metadata objects from a bucket under a prefix.
+
Review Comment:
this is optional. Can we add
`.. deprecated:: <version>`
Above will show up in docs. similar to [this](https://airflow.apache.org/docs/apache-airflow-providers-amazon/stable/_api/airflow/providers/amazon/aws/hooks/glue/index.html#airflow.providers.amazon.aws.hooks.glue.GlueJobHook.list_jobs)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3255837152
> > Actually sorry but I realized I provided wrong instructions. This does not solve back compat issues ... What we should do instead is deprecate `get_file_metadata` and create a new one using `Iterator` and use this one in the sensor. That way we do not need to create a major release because of this.
>
> I guess just wondering what is wrong with making a major release? How do we determine when to proceed with one?
Nothing wrong about it, we are just trying to minimize them. Upgrading major version can be painful for users and require some work so we are trying to do that not so often.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3254380844
Got it - so the plan would be to:
1. Deprecate `get_file_metadata` with a deprecation warning?
2. Create a new `get_file_metadata` method that returns an `Iterator`, and use that in the Sensor.
What is the best way to deprecate a method and create one with the same name?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2357996383
##########
providers/amazon/src/airflow/providers/amazon/aws/sensors/s3.py:
##########
@@ -122,8 +122,20 @@ def _check_key(self, key, context: Context):
"""
if self.wildcard_match:
prefix = re.split(r"[\[*?]", key, 1)[0]
- keys = self.hook.get_file_metadata(prefix, bucket_name)
- key_matches = [k for k in keys if fnmatch.fnmatch(k["Key"], key)]
+
+ key_matches: list[str] = []
+
+ # Is check_fn is None, then we can return True without having to iterate through each value in
+ # yielded by iter_file_metadata. Otherwise, we'll check for a match, and add all matches to the
+ # key_matches list
+ for k in self.hook.iter_file_metadata(prefix, bucket_name):
+ if fnmatch.fnmatch(k["Key"], key):
+ if self.check_fn is not None:
+ key_matches.append(k)
Review Comment:
I'm not familar with the code, but don't we need to call check_fn on k ?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2359620425
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -931,7 +932,51 @@ def get_file_metadata(
max_items: int | None = None,
) -> list:
"""
- List metadata objects in a bucket under prefix.
+ [DEPRECATED] Retrieve metadata objects from a bucket under a prefix.
+
Review Comment:
Done!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2359616145
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -931,7 +932,51 @@ def get_file_metadata(
max_items: int | None = None,
) -> list:
"""
- List metadata objects in a bucket under prefix.
+ [DEPRECATED] Retrieve metadata objects from a bucket under a prefix.
+
Review Comment:
Done.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3298710544
@eladkal, can you take a look at this for me?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2359843136
##########
providers/amazon/src/airflow/providers/amazon/aws/sensors/s3.py:
##########
@@ -122,8 +122,20 @@ def _check_key(self, key, context: Context):
"""
if self.wildcard_match:
prefix = re.split(r"[\[*?]", key, 1)[0]
- keys = self.hook.get_file_metadata(prefix, bucket_name)
- key_matches = [k for k in keys if fnmatch.fnmatch(k["Key"], key)]
+
+ key_matches: list[str] = []
+
+ # Is check_fn is None, then we can return True without having to iterate through each value in
+ # yielded by iter_file_metadata. Otherwise, we'll check for a match, and add all matches to the
+ # key_matches list
+ for k in self.hook.iter_file_metadata(prefix, bucket_name):
+ if fnmatch.fnmatch(k["Key"], key):
+ if self.check_fn is not None:
+ key_matches.append(k)
Review Comment:
Yeah for sure
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #55070:
URL: https://github.com/apache/airflow/pull/55070#issuecomment-3237183607
@eladkal, since we're changing the return type of `get_file_metadata`, would this need to be part of a major release?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reducing memory footprint for synchronous `S3KeySensor` [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on code in PR #55070:
URL: https://github.com/apache/airflow/pull/55070#discussion_r2310230114
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -957,11 +957,10 @@ def get_file_metadata(
params["RequestPayer"] = "requester"
response = paginator.paginate(**params)
- files = []
+ # Rather than returning a list and loading all keys into memory, we'll iterate over the results
Review Comment:
don't need this comment
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org