almalinux/cas_wrapper
13
0
Fork 2
Code Issues Pull Requests Releases Activity

ALBS-444 #2

Merged
anfimovdm merged 9 commits from ALBS-444 into master 2022-07-01 14:25:17 +00:00
Conversation 0 Commits 9 Files Changed 3 +108 -15
anfimovdm commented 2022-06-27 09:18:39 +00:00
Member
Copy Link
No description provided.
anfimovdm added 3 commits 2022-06-27 09:18:40 +00:00
anfimovdm added 1 commit 2022-06-28 19:06:45 +00:00
anfimovdm added 1 commit 2022-06-29 10:17:32 +00:00
AvoidMe reviewed 2022-06-29 16:04:37 +00:00
cas_wrapper.py Outdated
@ -96,0 +174,4 @@
self._logger.exception('Cannot notarize artifact:')
all_artifacts_is_notarized = False
continue
artifact.cas_hash = cas_artifact_hash
AvoidMe commented 2022-06-29 16:04:37 +00:00
First-time contributor
Copy Link

It's bad practice to mutate arguments like this, can you return all artifacts with the respected hashes instead?

It's bad practice to mutate arguments like this, can you return all artifacts with the respected hashes instead?
anfimovdm commented 2022-06-30 10:07:12 +00:00
Author
Member
Copy Link

Done

Done
anfimovdm marked this conversation as resolved
AvoidMe reviewed 2022-06-29 16:06:27 +00:00
cas_wrapper.py Outdated
@ -96,0 +144,4 @@
use_hash: bool = False,
) -> bool:
is_authenticated = False
with self as cas:
AvoidMe commented 2022-06-29 16:06:27 +00:00
First-time contributor
Copy Link

Instead of with self I would prefer self.ensure_login() or something, because you didn't use exit

Instead of `with self` I would prefer `self.ensure_login()` or something, because you didn't use __exit__
anfimovdm commented 2022-06-30 10:07:22 +00:00
Author
Member
Copy Link

Done

Done
anfimovdm marked this conversation as resolved
AvoidMe added 1 commit 2022-06-29 16:51:00 +00:00
anfimovdm added 1 commit 2022-06-30 10:06:41 +00:00
soksanichenko requested changes 2022-06-30 10:15:43 +00:00
soksanichenko left a comment
Owner
Copy Link

Add docstring to the new functions

Add docstring to the new functions
cas_wrapper.py
@ -96,0 +121,4 @@
# it should return 0 for authenticated and trusted commits
is_authenticated = not bool(
result_json.get('status', 1))
commit_cas_hash = result_json.get('hash')
soksanichenko commented 2022-06-30 10:13:18 +00:00
Owner
Copy Link

You don't get hash because use_hash equals to Fasle by default

You don't get hash because use_hash equals to Fasle by default
anfimovdm commented 2022-06-30 12:27:28 +00:00
Author
Member
Copy Link

I can get hash here, because when I use return_json flag, self.authenticate returns full JSON response

I can get hash here, because when I use `return_json` flag, `self.authenticate` returns full JSON response
anfimovdm marked this conversation as resolved
cas_wrapper.py Outdated
@ -96,0 +145,4 @@
# we can fall with ProcessExecutionError,
# because source can be not notarized
except ProcessExecutionError:
self._logger.exception('Cannot authenticate %s:', local_path)
soksanichenko commented 2022-06-30 10:14:21 +00:00
Owner
Copy Link

I guess colon is not needed in the logging message or it's in wrong place of the message

I guess colon is not needed in the logging message or it's in wrong place of the message
anfimovdm marked this conversation as resolved
cas_wrapper.py Outdated
@ -96,0 +166,4 @@
try:
cas_artifact_hash = future.result()
except Exception:
self._logger.exception('Cannot notarize artifact:')
soksanichenko commented 2022-06-30 10:15:24 +00:00
Owner
Copy Link

Add name of an artifact to the logging message

Add name of an artifact to the logging message
anfimovdm marked this conversation as resolved
anfimovdm added 1 commit 2022-06-30 13:06:03 +00:00
anfimovdm force-pushed ALBS-444 from 9f15b9b5be to fa16027062 2022-06-30 13:27:07 +00:00 Compare
Korulag approved these changes 2022-06-30 14:35:41 +00:00
soksanichenko approved these changes 2022-07-01 10:31:24 +00:00
anfimovdm merged commit 98ca413db7 into master 2022-07-01 14:25:17 +00:00
anfimovdm referenced this issue from a commit 2022-07-01 14:25:17 +00:00
ALBS-444 (#2)
Sign in to join this conversation.
Reviewers
No reviewers
Korulag
soksanichenko
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: almalinux/cas_wrapper#2
No description provided.
Delete Branch "ALBS-444"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?