2763 dont trigger telegram flow if no infographics#2764
2763 dont trigger telegram flow if no infographics#2764tkalir wants to merge 2 commits intodata-for-change:devfrom
Conversation
…ering dag if no widgets are found
…y_to_s3 and send_infographics_to_telegram
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #2764 +/- ##
==========================================
- Coverage 52.57% 52.44% -0.13%
==========================================
Files 122 122
Lines 10232 10256 +24
==========================================
Hits 5379 5379
- Misses 4853 4877 +24
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| else TELEGRAM_POST_VERIFICATION_CHANNEL_CHAT_ID | ||
| trigger_airflow_dag("generate-and-send-infographics-images", dag_conf) | ||
| infographics_data = get_infographics_data_by_newsflash(newsflash_id) | ||
| if WIDGETS not in infographics_data: |
There was a problem hiding this comment.
I suggest adding a years_ago param so we'll be able to adjust from get_infographics_data_by_newsflash (and default will be current default - BE_CONST.DEFAULT_NUMBER_OF_YEARS_AGO)
| dag_conf = {"news_flash_id": newsflash_id} | ||
| dag_conf["chat_id"] = TELEGRAM_CHANNEL_CHAT_ID if pre_verification_chat \ | ||
| else TELEGRAM_POST_VERIFICATION_CHANNEL_CHAT_ID | ||
| trigger_airflow_dag("generate-and-send-infographics-images", dag_conf) |
There was a problem hiding this comment.
I suggest adding the years_ago param to dag_conf to have this flexibility in this dag as well, can be useful in the future
|
@tkalir looks good! Added small comments and also Pylint tests need to be fixed. After that I suggest we'll merge |
3 participants
No description provided.