LITE-27895 Add create ppr functionality #41
Conversation
| @@ -12,20 +12,22 @@ class Configuration(Model): | |||
|
|
|||
| PREFIX = 'CFL' | |||
|
|
|||
| STATE = ConfigurationStateChoices | |||
There was a problem hiding this comment.
There is a lot of refactoring in several files. I think it is more convenient to make a separate commit for refactoring.
| @@ -68,7 +68,11 @@ class ConfigurationCreateSchema(NonNullSchema): | |||
| file: FileSchema | |||
|
|
|||
|
|
|||
| class ConfigurationSchema(NonNullSchema): | |||
| class PPRCreateSchema(NonNullSchema): | |||
There was a problem hiding this comment.
Description could be provided by user so it is required here. But I have it in my PR, so I will add it after merge.
| return new_version | ||
|
|
||
|
|
||
| def create_ppr(ppr, context, deployment, db, client, logger): |
There was a problem hiding this comment.
This function is very big and has a lot of logic. It might be more convenient to refactor it and to split to several separate functions.
There was a problem hiding this comment.
I agree here, at least there are two branches that can be done as separate functions --> uploading and generation
| summary.update({'errors': errors}) | ||
| status = PPRVersion.STATUS.failed | ||
| try: | ||
| file_instance = File( |
There was a problem hiding this comment.
I propose to leave this logic related to models in webapp.py for uniformity
There was a problem hiding this comment.
I agree on reduce the create function because is really big, but all related to file management, creating object is more related to the service layer than the webapp.
connect_ext_ppr/service.py
Outdated
| .filter_by( | ||
| deployment=deployment.id, | ||
| state=Configuration.STATE.active, | ||
| ).first() |
There was a problem hiding this comment.
Better to use .one_or_none(), I suppose, to catch situation if there are more then one active configuration.
| ) | ||
| items = list(get_product_items(client, deployment.product.id)) | ||
|
|
||
| file, writer, wb = get_base_workbook(data) |
There was a problem hiding this comment.
All this file operations might be moved to the separate function
| @@ -11,7 +11,7 @@ | |||
| <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin> | |||
| <link href="https://fonts.googleapis.com/css2?family=Roboto:ital,wght@0,400;0,500;0,700;1,400;1,500;1,700&display=swap" rel="stylesheet"> | |||
| <title>Index</title> | |||
| <script defer src="vendors.c37f1993399243c8d2a4.js"></script><script defer src="index.f0239835f73a360467d7.js"></script><link href="index.5c5477c3e81eb95e1456.css" rel="stylesheet"></head> | |||
| <script defer src="vendors.0b82e61f94dfa88a3bd2.js"></script><script defer src="index.acad834064fd73a90d98.js"></script><link href="index.5c5477c3e81eb95e1456.css" rel="stylesheet"></head> | |||
There was a problem hiding this comment.
Why do these changes occur? I have the same in my PRs. Maybe there is a way to prevent this?
connect_ext_ppr/service.py
Outdated
| f"(product_id={deployment.product_id}, " | ||
| f"product_version={deployment.product.version})" | ||
| ) | ||
| print(previous_ppr) |
| return new_version | ||
|
|
||
|
|
||
| def create_ppr(ppr, context, deployment, db, client, logger): |
There was a problem hiding this comment.
I agree here, at least there are two branches that can be done as separate functions --> uploading and generation
jonatrios
force-pushed
the
LITE-27895-add-create-ppr-functionality-for-regenerate-action
branch
from
265df88
to
9691966
Compare
jonatrios
force-pushed
the
LITE-27895-add-create-ppr-functionality-for-regenerate-action
branch
from
9691966
to
d64ac54
Compare
d3rky
deleted the
LITE-27895-add-create-ppr-functionality-for-regenerate-action
branch
Changes on this PR: