Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

renamed and moved ComponentStatus enum #15

Merged
merged 6 commits into from
Mar 29, 2023
Merged

renamed and moved ComponentStatus enum #15

merged 6 commits into from
Mar 29, 2023

Conversation

xtuned
Copy link
Contributor

@xtuned xtuned commented Mar 29, 2023

In this PR;

  • I renamed DownloadState to ComponentStatus
  • Moved ComponentStatus class to zpodcommon
  • Updated component enable function to return immediately if the download is already completed

tsugliani
tsugliani reviewed Mar 29, 2023
View reviewed changes
zpodapi/src/zpodapi/components/component__services.py Outdated
@@ -12,6 +12,9 @@ def get(self, *, value, column="component_uid"):
return super().get(value=value, column=column)

def enable(self, *, component: M.Component):

if component.status == CS.DOWNLOAD_COMPLETE and component.enabled is True:
return component
component.enabled = True
component.status = "SCHEDULED"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

component.status = CS.SCHEDULED I think no ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I missed that

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably CS.SCHEDULED.value, but test both

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

kvalenti
kvalenti reviewed Mar 29, 2023
View reviewed changes
zpodcommon/src/zpodcommon/enums.py Outdated
DOWNLOAD_COMPLETE = "DOWNLOAD_COMPLETE"
FAILED_AUTHENTICATION = "FAILED_AUTHENTICATION"
SCHEDULED = "SCHEDULED"
DOWNLOAD_INCOMPLETE = "DOWNLOAD_INCOMPLETE "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove extra space after DOWNLOAD_INCOMPLETE

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed the extra space

zpodapi/src/zpodapi/components/component__services.py Outdated
@@ -12,6 +12,9 @@ def get(self, *, value, column="component_uid"):
return super().get(value=value, column=column)

def enable(self, *, component: M.Component):

if component.status == CS.DOWNLOAD_COMPLETE and component.enabled is True:
return component
component.enabled = True
component.status = "SCHEDULED"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably CS.SCHEDULED.value, but test both

zpodapi/src/zpodapi/components/component__services.py Outdated
@@ -1,5 +1,5 @@
from sqlmodel import SQLModel

from zpodcommon.enums import ComponentStatus as CS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like isort wasn't applied here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

applied isort

zpodengine/src/zpodengine/component/flow_download_component.py
@@ -178,11 +170,11 @@ def download_component(component: Component) -> int:
return 0
except RuntimeError as e:
if e.args[0] == "AuthenticationError":
update_db(component.component_uid, DownloadState.FAILED_AUTHENTICATION.name)
update_db(component.component_uid, CS.FAILED_AUTHENTICATION)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need .value here?

zpodengine/src/zpodengine/component/flow_download_component.py Outdated
logger.error("The provided credentials are not correct.")
raise e
if e.args[0] == "EntitlementError":
update_db(component.component_uid, DownloadState.NOT_ENTITLED.name)
update_db(component.component_uid, CS.NOT_ENTITLED.name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need .value here instead of .name or remove .name?

zpodengine/src/zpodengine/component/flow_download_component.py Outdated
@@ -258,7 +250,7 @@ def verify_checksum(component: Component, filename: Path) -> bool:
if component.component_download_file_checksum is None:
if component.component_dl_path.exists():
update_db(
uid=component.component_uid, status=DownloadState.DOWNLOAD_COMPLETE.name
uid=component.component_uid, status=CS.DOWNLOAD_COMPLETE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need .value here?

zpodengine/src/zpodengine/component/flow_download_component.py Outdated
@@ -269,11 +261,11 @@ def verify_checksum(component: Component, filename: Path) -> bool:
logger.info(f"Checksum: {checksum}")
if checksum != expected_checksum:
update_db(
uid=component.component_uid, status=DownloadState.DOWNLOAD_INCOMPLETE.name
uid=component.component_uid, status=CS.DOWNLOAD_INCOMPLETE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need .value here?

zpodengine/src/zpodengine/component/flow_download_component.py Outdated
)
raise ValueError("Checksum does not match")
logger.info(f"Updating {component.component_uid} status")
update_db(uid=component.component_uid, status=DownloadState.DOWNLOAD_COMPLETE.name)
update_db(uid=component.component_uid, status=CS.DOWNLOAD_COMPLETE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need .value here?

zpodengine/src/zpodengine/component/flow_download_component.py
Comment on lines +320 to +323
CS.FAILED_AUTHENTICATION,
CS.FAILED_DOWNLOAD,
CS.NOT_ENTITLED,
CS.DOWNLOAD_COMPLETE,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need .value here?

Copy link
Contributor Author

@xtuned xtuned Mar 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I tested it and it worked just fine. Initially it was defined as int and needed string that's why I used the .name above

@kvalenti kvalenti merged commit 9d13dba into main Mar 29, 2023
@kvalenti kvalenti deleted the component-fix branch March 29, 2023 18:36
tsugliani pushed a commit that referenced this pull request Mar 26, 2024
@xtuned
renamed and moved ComponentStatus enum (#15)
b9038fe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsugliani tsugliani tsugliani left review comments

@kvalenti kvalenti kvalenti left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@xtuned @tsugliani @kvalenti