uranusjr commented on code in PR #60963:
URL: https://github.com/apache/airflow/pull/60963#discussion_r2730418029
##########
providers/imap/src/airflow/providers/imap/hooks/imap.py:
##########
@@ -240,12 +260,17 @@ def _retrieve_mails_attachments_by_name(
return all_matching_attachments
- def _list_mail_ids_desc(self, mail_filter: str) -> Iterable[str]:
+ def _list_mail_ids_desc(self, mail_filter: str, max_mails: int | None =
None) -> Iterable[str]:
if not self.mail_client:
raise RuntimeError("The 'mail_client' should be initialized
before!")
_, data = self.mail_client.search(None, mail_filter)
mail_ids = data[0].split()
- return reversed(mail_ids)
+ mail_ids_desc = reversed(mail_ids)
Review Comment:
I assume the motivation behind the new argument is for performance. However,
this `reversed` call would defeat half of it since the hook would need to spend
time ordering entries that are going be end up being dropped anyway.
I would recommend to instead implement logic that calculates the islice
arguments _before_ reversing, and then reverse the result afterwards. Using an
islice would also be unnecessary in this situation—sorting would impact
performance the most; if the number of entires is limited, a simple slice (or
using the `[:end]` syntax) would be sufficient.
--
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]
