Adds Globus Transfer support from storage endpoint #100
Conversation
cjh1
left a comment
cjh1
left a comment
There was a problem hiding this comment.
Looking good, some inline comments. Happy to discuss in person.
src/sfapi_client/_async/storage.py
Outdated
| source_uuid: str, | ||
| target_uuid: str, | ||
| source_dir: str, | ||
| target_dir: str, |
There was a problem hiding this comment.
We should allow <Async>RemotePath to be passed here so it integrates nicely with the other parts of the API. I think its just a question of updating typing, but we should add a test as well.
There was a problem hiding this comment.
Added in the typing here. The remote_file in the tests are <Async>RemotePath so it should be working as expected.
|
@cjh1 I think this should be set. I commented out a few integration tests that now fail because the status endpoint has changed. I think we'll need to do a real revamp of the status so I added an issue for it #102 I also ended up updating the python version to a minimum of 3.8 because of a few walrus operators that ruff found. Python 3.7 was EOL June 2023 so it's probably good not to support it anymore, we could also do the same for 3.8 which had it's EOL in October 2024 but at least the checks pass for 3.8 and the code should still work. |
The plan was to maintain API compatibility, I ran our tests before things were deployed, what tests are failing? |
|
The $ curl -X 'GET' 'https://api.nersc.gov/api/v1.2/status/notes' -H 'accept: application/json'
[[]] |
cjh1
left a comment
cjh1
left a comment
There was a problem hiding this comment.
LGTM, thanks for your persistence and changes!
No description provided.