-
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-28561 Implement sync PPR to marketplaces #117
LITE-28561 Implement sync PPR to marketplaces #117
Conversation
1e3bade
to
ad99d14
Compare
connect_ext_ppr/tasks_manager.py
Outdated
) | ||
.one_or_none() | ||
) | ||
file_data = get_ppr_from_media( |
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.
I have a question here. Should we wrap it with the try..except
block and raise a TaskException
?
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.
Yes, I think that would be better
connect_ext_ppr/utils.py
Outdated
else: | ||
ws[col] = 'FALSE' | ||
# This value updates here, because it is not convenient to return it | ||
marketplaces_to_not_update.update(cbc_marketplace_ids_dict.values()) |
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.
I suppose, it is a bad pattern to return value this way, but didn't come up with how to do it better. Any ideas?
ac81597
to
818c2e8
Compare
f7f614f
to
29c3eb9
Compare
29c3eb9
to
9ab65b3
Compare
connect_ext_ppr/tasks_manager.py
Outdated
) | ||
.one_or_none() | ||
) | ||
file_data = get_ppr_from_media( |
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.
Yes, I think that would be better
No description provided.