add checksum timestamp changes #371
Conversation
brynnz22
requested review from
eecavanna and
dwinston
and removed request for
eecavanna
nmdc_runtime/api/endpoints/util.py
Outdated
| @@ -442,6 +442,7 @@ def persist_content_and_get_drs_object( | |||
| ), | |||
| "access_methods": [{"access_id": drs_id}], | |||
| }, | |||
| timestamp= datetime.now(tz=ZoneInfo('America/Los_Angeles')).isoformat(timespec='minutes') | |||
There was a problem hiding this comment.
This change makes sense to me.
A string is being generated that indicates the current time (in Los Angeles, CA) down to the minute.
Docs for datetime.isoformat() say it returns a string representation of the time in ISO 8601 format.
There was a problem hiding this comment.
I recommend removing the space after the equals sign, since this is "keyword argument setting" as opposed to "variable assignment" (I think that's a Python convention).
- timestamp= datetime.now(...
+ timestamp=datetime.now(...
There was a problem hiding this comment.
The code changes, in isolation, make sense to me. I will defer to @dwinston for final approval and merging, as I'm not too familiar with the context (i.e. this part of the code base) and I think these changes stemmed from a comment he made in #340 (comment).
| @@ -431,17 +431,21 @@ def persist_content_and_get_drs_object( | |||
| filepath = str(Path(save_dir).joinpath(filename)) | |||
| with open(filepath, "w") as f: | |||
| f.write(content) | |||
| now_to_the_minute = datetime.now(tz=ZoneInfo("America/Los_Angeles")).isoformat( | |||
There was a problem hiding this comment.
style: creating this in one place
| TEST_FILES_DIR = Path(__file__).parent.parent.joinpath("files") | ||
|
|
||
|
|
||
| def test_sha256hash_from_file_is_timestamp_dependent(): |
#340
Hi @dwinston , I am unfamiliar with the
hashliblibrary, so I'm not sure about the code I added to thesha256hash_from_filefunction. I think this is what you meant.Thanks!