migrate `prefect_aws.s3` async tasks to `async_dispatch` #16096

zzstoatzz · 2024-11-22T21:03:16Z

related to #15008

adds explicit async methods to all public tasks and methods in prefect_aws.s3 and removes use of sync_compatable

old notes

i intentionally did not do the S3Bucket methods here, since there's an open question I want to discuss with @desertaxle

question

on S3Bucket

async def get_directory()

is not actually async at all, not even a sending sync stuff to a worker thread

the established pattern would say we do

async def aget_directory(self):
   # send to worker threads

@async_dispatch(aget_directory)
def get_directory(self):
   # original impl

that seems like it was duplicate a lot for not a very compelling reason

but because stopped supporting async in sync we might need to do this

also renamed tasks in backword compat fashion per this comment

src/integrations/prefect-aws/prefect_aws/s3.py

desertaxle

Couple of nits on docstring examples. Also, it'd be good to have some tests covering the new async functions and methods.

src/integrations/prefect-aws/prefect_aws/s3.py

zzstoatzz · 2024-11-25T20:29:50Z

updated the docstrings and added some coverage for the new async tasks

discdiver

Nice! I added a note for each of the new/changed methods so that future SDK docs will reflect when a method was added/changed.

I assume a minor version bump is coming, but please change the versions noted if not.

discdiver · 2024-11-25T21:07:26Z

src/integrations/prefect-aws/prefect_aws/s3.py

+    key: Optional[str] = None,
+) -> str:
+    """
+    Uploads data to an S3 bucket.


Suggested change

Uploads data to an S3 bucket.

Uploads data to an S3 bucket. Added in version 0.5.3.

@discdiver do you think we should have a minor version bump because of the new async versions of tasks?

I'd vote for it.

We reserve minor version bumps in integration packages for breaking changes, so we should not release a new minor version for these changes.

Cool. Then just 0.5.3 for each note. I can edit the suggestions.

discdiver · 2024-11-25T21:08:25Z

src/integrations/prefect-aws/prefect_aws/s3.py

+    aws_client_parameters: AwsClientParameters = AwsClientParameters(),
+) -> bytes:
+    """
+    Downloads an object with a given key from a given S3 bucket.


Suggested change

Downloads an object with a given key from a given S3 bucket.

Downloads an object with a given key from a given S3 bucket.

Added in version 0.5.3.

Or whichever is the next version if not a minor bump.

discdiver · 2024-11-25T21:10:43Z

src/integrations/prefect-aws/prefect_aws/s3.py

+    the credentials must have permission to read and delete the source object and write
+    to the target object. If the credentials do not have those permissions, this method
+    will raise an error. If the credentials have permission to read the source object
+    but not delete it, the object will be copied but not deleted.


Suggested change

but not delete it, the object will be copied but not deleted.

but not delete it, the object will be copied but not deleted.

Added in version 0.5.3.

discdiver · 2024-11-25T21:11:02Z

src/integrations/prefect-aws/prefect_aws/s3.py

+
+def _list_objects_sync(page_iterator: PageIterator):
+    """
+    Synchronous method to collect S3 objects into a list


Suggested change

Synchronous method to collect S3 objects into a list

Synchronous method to collect S3 objects into a list.

Added in version 0.5.3.

src/integrations/prefect-aws/prefect_aws/s3.py

discdiver · 2024-11-25T21:17:13Z

src/integrations/prefect-aws/prefect_aws/s3.py

        self,
        from_folder: str,
        to_folder: Optional[Union[str, Path]] = None,
        **download_kwargs: Dict[str, Any],
    ) -> Path:
        """
-        Downloads objects *within* a folder (excluding the folder itself)
+        Asynchronously downloads objects *within* a folder (excluding the folder itself)
        from the S3 bucket to a folder.


Suggested change

from the S3 bucket to a folder.

from the S3 bucket to a folder. Changed in version 0.5.3.

src/integrations/prefect-aws/prefect_aws/s3.py

zzstoatzz · 2024-11-26T13:50:45Z

@discdiver I'm not sure I like the idea of hardcoding versions where things in appeared in docstrings. also, nothing here is "now asynchronous", there are new tasks which are asynchronous, and the sync tasks will work the same

cc @desertaxle on the docstring question

discdiver · 2024-11-26T15:54:46Z

@discdiver I'm not sure I like the idea of hardcoding versions where things in appeared in docstrings. also, nothing here is "now asynchronous", there are new tasks which are asynchronous, and the sync tasks will work the same

cc @desertaxle on the docstring question

Thanks @zzstoatzz. Removed note on async. The official Python docs use an .rst directive and then the version change note only shows up in the docs and not the inline docstring. I've asked the Mintlify team if they could add a keyword or symbol that did something similar.

I can see arguments both ways. It's nice to keep the docstrings uncluttered. And the note feels pretty small and could be helpful in some cases.

zzstoatzz · 2024-11-26T15:59:42Z

And the note feels pretty small and could be helpful in some cases.

fair point! I think its a fine compromise to have them in there 👍

thank you for adding them!

Co-authored-by: Jeff Hale <[email protected]>

github-actions bot added the enhancement An improvement of an existing feature label Nov 23, 2024

desertaxle reviewed Nov 25, 2024

View reviewed changes

src/integrations/prefect-aws/prefect_aws/s3.py Outdated Show resolved Hide resolved

zzstoatzz force-pushed the async-dispatch-aws-s3 branch from 6fa4f6b to 42ac696 Compare November 25, 2024 15:12

zzstoatzz marked this pull request as ready for review November 25, 2024 15:13

zzstoatzz requested a review from cicdw as a code owner November 25, 2024 15:13

zzstoatzz requested a review from desertaxle November 25, 2024 15:52

desertaxle reviewed Nov 25, 2024

View reviewed changes

zzstoatzz changed the title ~~migrate prefect_aws.s3 async tasks to async_dispatch pt1~~ migrate prefect_aws.s3 async tasks to async_dispatch Nov 25, 2024

zzstoatzz requested a review from desertaxle November 25, 2024 21:47

discdiver reviewed Nov 25, 2024

View reviewed changes

zzstoatzz and others added 11 commits November 26, 2024 10:12

pt1

a118398

do s3 bucket too

78be007

update names and preserve backwards compat

e04021c

basic coverage for async functions

f136cad

remove unnecessary kwarg

5a5879e

Update src/integrations/prefect-aws/prefect_aws/s3.py

2f64122

Co-authored-by: Jeff Hale <[email protected]>

Update src/integrations/prefect-aws/prefect_aws/s3.py

8d72347

Co-authored-by: Jeff Hale <[email protected]>

Update src/integrations/prefect-aws/prefect_aws/s3.py

33823b2

Co-authored-by: Jeff Hale <[email protected]>

Update src/integrations/prefect-aws/prefect_aws/s3.py

0091bd0

Co-authored-by: Jeff Hale <[email protected]>

Update src/integrations/prefect-aws/prefect_aws/s3.py

942b869

Co-authored-by: Jeff Hale <[email protected]>

add note on version

5df582e

zzstoatzz force-pushed the async-dispatch-aws-s3 branch from 62d0ee8 to 5df582e Compare November 26, 2024 16:12

desertaxle approved these changes Nov 26, 2024

View reviewed changes

zzstoatzz merged commit f58d064 into main Nov 26, 2024
12 checks passed

zzstoatzz deleted the async-dispatch-aws-s3 branch November 26, 2024 16:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

migrate `prefect_aws.s3` async tasks to `async_dispatch` #16096

migrate `prefect_aws.s3` async tasks to `async_dispatch` #16096

zzstoatzz commented Nov 22, 2024 •

edited

Loading

desertaxle left a comment

zzstoatzz commented Nov 25, 2024

discdiver left a comment

discdiver Nov 25, 2024 •

edited

Loading

zzstoatzz Nov 25, 2024

discdiver Nov 25, 2024

desertaxle Nov 26, 2024

discdiver Nov 26, 2024

discdiver Nov 25, 2024 •

edited

Loading

discdiver Nov 25, 2024

discdiver Nov 25, 2024 •

edited

Loading

discdiver Nov 25, 2024 •

edited

Loading

discdiver Nov 25, 2024 •

edited by zzstoatzz

Loading

zzstoatzz commented Nov 26, 2024

discdiver commented Nov 26, 2024

zzstoatzz commented Nov 26, 2024 •

edited

Loading

	Uploads data to an S3 bucket.
	Uploads data to an S3 bucket. Added in version 0.5.3.

	Downloads an object with a given key from a given S3 bucket.
	Downloads an object with a given key from a given S3 bucket.
	Added in version 0.5.3.

	but not delete it, the object will be copied but not deleted.
	but not delete it, the object will be copied but not deleted.
	Added in version 0.5.3.

	Synchronous method to collect S3 objects into a list
	Synchronous method to collect S3 objects into a list.
	Added in version 0.5.3.

	from the S3 bucket to a folder.
	from the S3 bucket to a folder. Changed in version 0.5.3.

migrate prefect_aws.s3 async tasks to async_dispatch #16096

migrate prefect_aws.s3 async tasks to async_dispatch #16096

Conversation

zzstoatzz commented Nov 22, 2024 • edited Loading

question

desertaxle left a comment

Choose a reason for hiding this comment

zzstoatzz commented Nov 25, 2024

discdiver left a comment

Choose a reason for hiding this comment

discdiver Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

zzstoatzz Nov 25, 2024

Choose a reason for hiding this comment

discdiver Nov 25, 2024

Choose a reason for hiding this comment

desertaxle Nov 26, 2024

Choose a reason for hiding this comment

discdiver Nov 26, 2024

Choose a reason for hiding this comment

discdiver Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

discdiver Nov 25, 2024

Choose a reason for hiding this comment

discdiver Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

discdiver Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

discdiver Nov 25, 2024 • edited by zzstoatzz Loading

Choose a reason for hiding this comment

zzstoatzz commented Nov 26, 2024

discdiver commented Nov 26, 2024

zzstoatzz commented Nov 26, 2024 • edited Loading

migrate `prefect_aws.s3` async tasks to `async_dispatch` #16096

migrate `prefect_aws.s3` async tasks to `async_dispatch` #16096

zzstoatzz commented Nov 22, 2024 •

edited

Loading

discdiver Nov 25, 2024 •

edited

Loading

discdiver Nov 25, 2024 •

edited

Loading

discdiver Nov 25, 2024 •

edited

Loading

discdiver Nov 25, 2024 •

edited

Loading

discdiver Nov 25, 2024 •

edited by zzstoatzz

Loading

zzstoatzz commented Nov 26, 2024 •

edited

Loading