ALBS-444 #2

Merged
anfimovdm merged 9 commits from ALBS-444 into master 2022-07-01 14:25:17 +00:00
2 changed files with 33 additions and 41 deletions
Showing only changes of commit e5b832a30a - Show all commits

View File

@ -4,12 +4,6 @@ import logging
import typing import typing
from plumbum import local, ProcessExecutionError from plumbum import local, ProcessExecutionError
from pydantic import BaseModel
class CasArtifact(BaseModel):
path: str
cas_hash: typing.Optional[str]
class CasWrapper: class CasWrapper:
@ -37,16 +31,12 @@ class CasWrapper:
if self._logger is None: if self._logger is None:
self._logger = logging.getLogger() self._logger = logging.getLogger()
def __enter__(self): def ensure_login(self):
with local.env( with local.env(
CAS_API_KEY=self._cas_api_key, CAS_API_KEY=self._cas_api_key,
SIGNER_ID=self._cas_signer_id, SIGNER_ID=self._cas_signer_id,
): ):
self._cas['login']() self._cas['login']()
return self
def __exit__(self, exc_type, value, traceback):
pass
def notarize( def notarize(
self, self,
@ -125,17 +115,17 @@ class CasWrapper:
) -> typing.Tuple[bool, typing.Optional[str]]: ) -> typing.Tuple[bool, typing.Optional[str]]:
is_authenticated = False is_authenticated = False
commit_cas_hash = None commit_cas_hash = None
with self as cas: self.ensure_login()
try: try:
result_json = cas.authenticate(local_path, return_json=True) result_json = self.authenticate(local_path, return_json=True)
# it should return 0 for authenticated and trusted commits # it should return 0 for authenticated and trusted commits
is_authenticated = not bool( is_authenticated = not bool(
result_json.get('status', 1)) result_json.get('status', 1))
commit_cas_hash = result_json.get('hash') commit_cas_hash = result_json.get('hash')
anfimovdm marked this conversation as resolved
Review

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
Review

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
# we can fall with ProcessExecutionError, # we can fall with ProcessExecutionError,
# because source can be not notarized # because source can be not notarized
except ProcessExecutionError: except ProcessExecutionError:
self._logger.exception('Cannot authenticate %s:', local_path) self._logger.exception('Cannot authenticate %s:', local_path)
return is_authenticated, commit_cas_hash return is_authenticated, commit_cas_hash
def authenticate_artifact( def authenticate_artifact(
@ -145,36 +135,39 @@ class CasWrapper:
return_json: bool = False, return_json: bool = False,
) -> bool: ) -> bool:
is_authenticated = False is_authenticated = False
with self as cas: self.ensure_login()
try: try:
is_authenticated = cas.authenticate(local_path, is_authenticated = self.authenticate(
use_hash=use_hash, local_path,
return_json=return_json) use_hash=use_hash,
# we can fall with ProcessExecutionError, return_json=return_json,
# because source can be not notarized )
except ProcessExecutionError: # we can fall with ProcessExecutionError,
self._logger.exception('Cannot authenticate %s:', local_path) # because source can be not notarized
except ProcessExecutionError:
anfimovdm marked this conversation as resolved Outdated

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__

Done

Done
self._logger.exception('Cannot authenticate %s:', local_path)
anfimovdm marked this conversation as resolved Outdated

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
return is_authenticated return is_authenticated
def notarize_artifacts( def notarize_artifacts(
self, self,
artifacts: typing.List[CasArtifact], artifact_paths: typing.List[str],
metadata: typing.Dict[str, typing.Any], metadata: typing.Dict[str, typing.Any],
) -> bool: ) -> typing.Tuple[bool, typing.Dict[str, str]]:
all_artifacts_is_notarized = True all_artifacts_is_notarized = True
with self as cas, ThreadPoolExecutor(max_workers=4) as executor: notarized_artifacts = {}
self.ensure_login()
with ThreadPoolExecutor(max_workers=4) as executor:
futures = { futures = {
executor.submit(cas.notarize, artifact.path, metadata): artifact executor.submit(self.notarize, artifact_path, metadata): artifact_path
for artifact in artifacts for artifact_path in artifact_paths
if not artifact.cas_hash
} }
for future in as_completed(futures): for future in as_completed(futures):
artifact = futures[future] artifact_path = futures[future]
try: try:
cas_artifact_hash = future.result() cas_artifact_hash = future.result()
except Exception: except Exception:
self._logger.exception('Cannot notarize artifact:') self._logger.exception('Cannot notarize artifact:')
anfimovdm marked this conversation as resolved Outdated

Add name of an artifact to the logging message

Add name of an artifact to the logging message
all_artifacts_is_notarized = False all_artifacts_is_notarized = False
continue continue
artifact.cas_hash = cas_artifact_hash notarized_artifacts[artifact_path] = cas_artifact_hash
return all_artifacts_is_notarized return all_artifacts_is_notarized, notarized_artifacts

View File

@ -21,7 +21,6 @@ setup(
scripts=['cas_wrapper.py'], scripts=['cas_wrapper.py'],
install_requires=[ install_requires=[
'plumbum>=1.7.2', 'plumbum>=1.7.2',
'pydantic>=1.8.1',
], ],
python_requires=">=3.6", python_requires=">=3.6",
) )