rewrite cp #498
rewrite cp #498
Conversation
Signed-off-by: Ayush Kamat <ayush@latch.bio>
Signed-off-by: Ayush Kamat <ayush@latch.bio>
| try: | ||
| parent.mkdir(exist_ok=True, parents=True) | ||
| break | ||
| except NotADirectoryError: # somewhere up the tree is a file |
There was a problem hiding this comment.
huh shouldnt this eat shit?
There was a problem hiding this comment.
not sure what this means
latch_cli/services/cp/main.py
Outdated
| acc_info = execute(gql.gql(""" | ||
| query AccountInfo { | ||
| accountInfoCurrent { | ||
| id | ||
| } | ||
| } | ||
| """))["accountInfoCurrent"] | ||
|
|
||
| for src in srcs: | ||
| src_remote = is_remote_path(src) | ||
| acc_id = acc_info["id"] |
There was a problem hiding this comment.
what is the purpose of this?
There was a problem hiding this comment.
used in the get_path_error fn in the except for nice error message printing
There was a problem hiding this comment.
I would definitely only do this query if we need to (if there is an error)
There was a problem hiding this comment.
I would entirely avoid network stuff in the error path if possible.
| # jitter to not dos nuc-data | ||
| await asyncio.sleep(0.1 * random.random()) |
There was a problem hiding this comment.
kinda odd what did we see here before?
There was a problem hiding this comment.
for wide directories with small files theres not enough time between start-upload calls so we end up throttling nuc-data
There was a problem hiding this comment.
But like shouldn't we use a semaphore on the call rather than adding jitter? Jitter is worse because it is not aware of how many calls are inflight or how long they are taking.
| # exception handling | ||
| resp = await sess.post( | ||
| "https://nucleus.latch.bio/ldata/end-upload", | ||
| headers={"Authorization": get_auth_header()}, | ||
| json={ | ||
| "path": work.dest, | ||
| "upload_id": data["upload_id"], | ||
| "parts": [ | ||
| { | ||
| "ETag": part.etag, | ||
| "PartNumber": part.part, | ||
| } | ||
| for part in parts | ||
| ], | ||
| }, | ||
| ) | ||
| resp.raise_for_status() | ||
|
|
||
| if print_file_on_completion: | ||
| pbar.write(work.src.name) | ||
|
|
||
| pbar.reset() | ||
| total_pbar.update(1) | ||
|
|
||
| pbar.clear() | ||
|
|
There was a problem hiding this comment.
you might want to do a smarter backoff with more tries given that we can 429 on this
There was a problem hiding this comment.
makes sense for more retries - two qs:
- what is a smarter backoff method - im not super familiar with any other than exponential
- what does this backoff method lack that a smarter method would address?
There was a problem hiding this comment.
this one is kinda unbounded in retries but there should be a maximum. Ideally, we have a semaphore which bounds the number of concurrent calls to nuc-data and then backoffs are less important and we can keep as is.
Signed-off-by: Ayush Kamat <ayush@latch.bio>
Signed-off-by: Ayush Kamat <ayush@latch.bio>
Signed-off-by: Ayush Kamat <ayush@latch.bio>
latch_cli/services/cp/http_utils.py
Outdated
| "https://nucleus.latch.bio/ldata/start-upload": asyncio.BoundedSemaphore(2), | ||
| "https://nucleus.latch.bio/ldata/end-upload": asyncio.BoundedSemaphore(2), | ||
| } |
There was a problem hiding this comment.
u r probably fine with like 5 or 10 each
| start_upload_sema = asyncio.BoundedSemaphore(2) | ||
| end_upload_sema = asyncio.BoundedSemaphore(2) |
| if resp.status == 429: | ||
| raise RateLimitExceeded( | ||
| "The service is currently under load and could not complete your" | ||
| " request - please try again later." | ||
| ) |
There was a problem hiding this comment.
wait this should just backoff and retry? why are we failing here?
| if resp.status == 429: | ||
| raise RateLimitExceeded( | ||
| "The service is currently under load and could not complete your" | ||
| " request - please try again later." | ||
| ) |
Signed-off-by: Ayush Kamat <ayush@latch.bio>
No description provided.