Skip to content

Conversation

@plietar
Copy link
Member

@plietar plietar commented Apr 10, 2024

This is equivalent to orderly2's orderly_shared_resource. It looks for files in the shared directory, at the root of the repository. Files and directories are copied to the packet directory, and recorded in the metadata.

@plietar plietar requested a review from r-ash April 10, 2024 16:25
@plietar plietar force-pushed the mrc-4572 branch 2 times, most recently from 9cff8ad to 55cfd5d Compare April 11, 2024 19:20
Copy link
Contributor

@r-ash r-ash 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 great to me, added @richfitz to see if he wants to have a look too

@r-ash r-ash requested a review from richfitz April 12, 2024 10:26
elif src.is_dir():
shutil.copytree(src, dst, dirs_exist_ok=True)
copied = {
os.path.join(here, f): os.path.join(there, f)
Copy link
Member

Choose a reason for hiding this comment

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

Are we not moving to pathlib everywhere? Also happy to sort this out later in a single push too

Copy link
Member Author

@plietar plietar Apr 16, 2024

Choose a reason for hiding this comment

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

This is the dictionary that ends up being returned to the caller / used in the packet metadata. I think keeping them as str is more appropriate. Path isn't json-serializable natively, so we would have to cast it back as str anyway.

plietar added 11 commits April 19, 2024 17:11
This is equivalent to orderly2's orderly_shared_resource.  It looks for
files in the `shared` directory, at the root of the repository. Files
and directories are copied to the packet directory, and recorded in the
metadata.
#
# See https://github.com/python/cpython/issues/44626 for some discussion.
# Unfortunately, while the issue was closed, the `is_relative` function
# mentioned was never added.
Copy link
Member

Choose a reason for hiding this comment

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

Looks like in 3.13+ we can use this! python/cpython#113829 but for now this seems sensible

@plietar plietar merged commit 02f9114 into main Apr 24, 2024
@plietar plietar deleted the mrc-4572 branch May 13, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants