#704 check purchase #709
#704 check purchase #709
Conversation
shopelectro/report.py
Outdated
| from telegram import Bot | ||
|
|
||
|
|
||
| class TGReport: |
There was a problem hiding this comment.
Let's avoid abbreviation and call it TelegramReport
shopelectro/settings/base.py
Outdated
|
|
||
| TG_BOT_TOKEN = os.environ.get('TG_BOT_TOKEN') | ||
| TG_REPORT_ADDRESSEES = os.environ.get( | ||
| 'TG_REPORT_ADDRESSEE', '@artemiy312,@duker33' |
There was a problem hiding this comment.
| 'TG_REPORT_ADDRESSEE', '@artemiy312,@duker33' | |
| 'TG_REPORT_ADDRESSEES', '@artemiy312,@duker33' |
There was a problem hiding this comment.
let's plz create some channel "shopelectro_health" or "shopeletro_report" and send stats there
| @@ -61,18 +63,30 @@ def update_catalog(): | |||
| collect_static() | |||
| ] | |||
|
|
|||
| # @todo #690:60m Handle errors in check_purchase. | |||
| # Report failed attempts. Schedule it in the celery beat. | |||
| # @todo #690:30m Schedule check_purchase in the celery beat. | |||
There was a problem hiding this comment.
@artemiy312 , i already suggested asyncio instead of celery beat at the parent issue's PR:
#695 (comment)
What do you think about it?
There was a problem hiding this comment.
I believe it is bad idea:
- Django is a synchronous framework that will interfere with asynchronous code
- We need a stateful pipeline for message delivery to be able to save messages and process them later in case of deployment, bad internet connection, hardware problems, any other things that prevent a message processing (with celery we saved a lot of emails in such cases)
- We already have working solution, that looks fine and aren't annoying us
There was a problem hiding this comment.
i'll answer just for history:
- it's not big deal, i think. We can handle our async utils process it with our own separated module
- we can provide safety by hands in every concrete case. In orders case we can (and we do) put them to the db synchronously and then do some dangerous async operations
- we had problems with celery not long time ago: unsent mails, not launched celery services and so on. Problems for example: one, two, three
| success_page.load() | ||
| assert success_page.is_success() | ||
| except (WebDriverException, AssertionError) as err: | ||
| if self.request.retries + 1 > self.max_retries: |
There was a problem hiding this comment.
one failed retry seems to me enough reason to send the report. Why you choose three ones?
And we should send one report for one failing. With this code celery will spawn our telegram channel
There was a problem hiding this comment.
one failed retry seems to me enough reason to send the report
but what if it is just false positive temporary issue, that usually occurs.
just in case i will create the setting variable for retry number
With this code celery will spawn our telegram channel
It's not, it will only send the message on the last attempt.
There was a problem hiding this comment.
@artemiy312 , but this last attempt occurs every max_retries check. It increases an interval of repeated messages, but still called spawn =)
but what if it is just false positive temporary issue, that usually occurs.
it's not good, that it occurs. Tests on CI works fine. You can create setting var of course. But this problem requires more systematic decision
There was a problem hiding this comment.
@duker33 ,
but this last attempt occurs every max_retries check. It increases an interval of repeated messages, but still called spawn
i don't see the point. Can you describe it in more detail?
I believe it will report an error once at the end of the retry counter.
e.g. if max_retries = 3 then the third failed try only will be reported, and previous once will be omitted
But this problem requires more systematic decision
Let's just set max retries number to 1 and if there are issues with this, we will find a solution
There was a problem hiding this comment.
Let's just set max retries number to 1 and if there are issues with this, we will find a solution
@artemiy312 , y, it's perfect.
About max_retries - merge this code plz. I'll create issue and we'll discuss it separately
Closes #704