Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix deployment #169

Merged
merged 7 commits into from
Dec 7, 2022
Merged

Fix deployment #169

merged 7 commits into from
Dec 7, 2022

Conversation

ahuang11
Copy link
Contributor

@ahuang11 ahuang11 commented Dec 6, 2022

Closes PrefectHQ/prefect#7583 and #161

Apparently, cast_pathlib does not get called when you do self.storage.basepath so this makes type str and defaults to "" instead of None.

I was initially planning to submit a PR into Prefect to update: https://github.com/PrefectHQ/prefect/blob/main/src/prefect/deployments.py#L293-L309. However, I realized that since the concept of basepath in Prefect S3 is s3://--not the base path of the path and is never a pathlib.Path as in prefect-aws, I think it's more suitable for the PR to be here? Let me know if that makes sense.

Example

Screenshots

Checklist

  • References any related issue by including "Closes #" or "Closes ".
    • If no issue exists and your change is not a small fix, please create an issue first.
  • Includes tests or only affects documentation.
  • Passes pre-commit checks.
    • Run pre-commit install && pre-commit run --all locally for formatting and linting.
  • Includes screenshots of documentation updates.
    • Run mkdocs serve view documentation locally.
  • Summarizes PR's changes in CHANGELOG.md

Copy link
Member

@desertaxle desertaxle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it could be a breaking change. Can we verify if this still works if you provide a Path for basepath?

@ahuang11
Copy link
Contributor Author

ahuang11 commented Dec 6, 2022

Hmm yeah you're right; I thought Pydantic would simply coerce into a string. Should I submit a PR to Prefect to fix then?
image

@desertaxle
Copy link
Member

Would this work?

    basepath: Optional[Union[str, Path]] = Field(
        default="",
        description="Location to write to and read from in the S3 bucket. Defaults to "
        "the root of the bucket.",
    )

@ahuang11
Copy link
Contributor Author

ahuang11 commented Dec 6, 2022

Okay that works!

from pathlib import Path
from prefect_aws import AwsCredentials
from prefect_aws.s3 import S3Bucket

aws_credentials = await AwsCredentials.load("my-credentials")
bucket = S3Bucket(bucket_name="andrew-int-test-cmd", aws_credentials=aws_credentials, basepath=Path("my/home/path"))
await bucket.save("test")
prefect deployment build ./log_flow.py:log_flow -n log-flow-s3 -sb s3-bucket/test

image

@ahuang11 ahuang11 merged commit 99d77fe into main Dec 7, 2022
@ahuang11 ahuang11 deleted the fix_deployment branch December 7, 2022 21:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deployment to s3-bucket: PosixPath error
2 participants