-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
LITE-28651: Adding common methods to send ppr and check task status f… #103
LITE-28651: Adding common methods to send ppr and check task status f… #103
Conversation
tests/test_tasks_manager.py
Outdated
cbc_service = mocker.Mock() | ||
cbc_service.parse_ppr.return_value = parse_ppr_success_response | ||
cbc_service.apply_ppr.return_value = 100 | ||
_send_ppr(cbc_service, sample_ppr_file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May add assert _send_ppr(cbc_service, sample_ppr_file) == 100
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated!
364c835
to
49d571e
Compare
connect_ext_ppr/models/task.py
Outdated
@@ -33,6 +34,8 @@ class Task(Model): | |||
aborted_at = db.Column(db.DateTime(), nullable=True) | |||
aborted_by = db.Column(db.String(20), nullable=True) | |||
|
|||
dr_instance = relationship(DeploymentRequest, foreign_keys="Task.deployment_request") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[COD-C] In MarketplaceConfiguration
we have the following approach:
class MarketplaceConfiguration:
...
ppr_id = db.Column(db.String, db.ForeignKey(PPRVersion.id))
...
ppr = relationship('PPRVersion', foreign_keys='MarketplaceConfiguration.ppr_id')
I would use the same approach here. Whichever option we choose, I would suggest using the same format for all relations in models.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed offline. Changes applied.
tracking_id = cbc_service.apply_ppr(file) | ||
|
||
if not tracking_id: | ||
raise TaskException('Some error ocurred trying to upload ppr.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[COD-C] Are there any chances to put the error here into the exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
discussed offline.
connect_ext_ppr/tasks_manager.py
Outdated
|
||
|
||
def _check_cbc_task_status(cbc_service, tracking_id): | ||
task_log = cbc_service.search_task_logs_by_name(tracking_id)[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[COD-C] Dear Agos. Can you please check that for all the HTTP requests to commerce here there is a retry mechanism? The reason is that the connection always unstable there and it will decrease number of errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated!
49d571e
to
deb2504
Compare
…rom task manager.
deb2504
to
1d0eafb
Compare
…rom task manager.