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

Fix S3Bucket.copy_object target path resolution #385

Merged
merged 8 commits into from
Mar 25, 2024

Conversation

dominictarro
Copy link
Contributor

@dominictarro dominictarro commented Feb 24, 2024

Closes #384

Uses other bucket's _resolve_path method when copying to another S3Bucket object. If to_bucket is a string, doesn't resolve. Copies to self when to_bucket is None.

Example

from prefect_aws import S3Bucket

source = S3Bucket(bucket_name="source-bucket")
target = S3Bucket(bucket_name="target-bucket", bucket_folder="subpath/")
source.copy_object("abc.txt", "copies/abc.txt", target)
# subpath/copies/abc.txt

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.

@dominictarro dominictarro requested a review from a team as a code owner February 24, 2024 04:09
@dominictarro
Copy link
Contributor Author

Unclear what's up with the error message. Did something break in main?

AttributeError: 'CallSpec2' object has no attribute 'funcargs'

@zzstoatzz
Copy link
Contributor

zzstoatzz commented Feb 27, 2024

hey @dominictarro - that's because we had an issue with pytest. that change is merged now and we should be back to normal here. thanks for catching!

@dominictarro
Copy link
Contributor Author

I just noticed that the same issue is in the move methods, so don't merge yet. Going to revisit this when I have time.

@dominictarro
Copy link
Contributor Author

This is ready for review.

Copy link
Contributor

@zzstoatzz zzstoatzz left a comment

Choose a reason for hiding this comment

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

nice - thank you @dominictarro. this looks good to me

@zzstoatzz zzstoatzz merged commit 1b25b0a into PrefectHQ:main Mar 25, 2024
6 checks passed
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.

S3Bucket.copy_object: Only resolve target path with self if to_bucket is not defined
2 participants