-
Notifications
You must be signed in to change notification settings - Fork 244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2763 dont trigger telegram flow if no infographics #2764
base: dev
Are you sure you want to change the base?
2763 dont trigger telegram flow if no infographics #2764
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
No description provided.