-
Notifications
You must be signed in to change notification settings - Fork 299
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
Telegram long polling #2250
Telegram long polling #2250
Conversation
👀 |
@alexintech thanks for the contribution (again 😄). I'm not very familiar w/ Telegram. I'll ask around and see if someone else more familiar can review this. |
Hi everyone. what about this MR? |
Hi everyone. |
Hi! |
# Conflicts: # CHANGELOG.md # engine/apps/base/utils.py # helm/oncall/tests/telegram_env_test.yaml # helm/oncall/tests/wait_for_db_test.yaml # helm/oncall/values.yaml
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.
Looks good to me 👍 , minor comments added.
Maybe @iskhakov can take a look at the helm updates? (they make sense to me, but to get another couple of eyes)
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.
@matiasb helm changes lgtm 👍
@alexintech thanks for this contribution 🙂 some minor comments. you'll need to re-merge dev
into your branch to resolve some recent merge conflicts.
- containsDocument: | ||
kind: Deployment | ||
apiVersion: apps/v1 | ||
name: oncall-telegram-polling | ||
- isSubset: | ||
path: metadata.labels | ||
content: | ||
app.kubernetes.io/component: telegram-polling | ||
app.kubernetes.io/instance: oncall | ||
app.kubernetes.io/name: oncall | ||
- isSubset: | ||
path: spec.selector.matchLabels | ||
content: | ||
app.kubernetes.io/component: telegram-polling | ||
app.kubernetes.io/instance: oncall | ||
app.kubernetes.io/name: oncall | ||
- isSubset: | ||
path: spec.template.metadata.labels | ||
content: | ||
app.kubernetes.io/component: telegram-polling | ||
app.kubernetes.io/instance: oncall | ||
app.kubernetes.io/name: oncall | ||
# Should contain only one replica to avoid Conflict while polling Telegram updates | ||
- equal: | ||
path: spec.replicas | ||
value: 1 | ||
- equal: | ||
path: spec.template.spec.serviceAccountName | ||
value: oncall | ||
- contains: | ||
path: spec.template.spec.initContainers | ||
content: | ||
name: wait-for-db | ||
any: true | ||
- equal: | ||
path: spec.template.spec.containers[0].name | ||
value: telegram-polling | ||
- equal: | ||
path: spec.template.spec.containers[0].command | ||
value: ['sh', '-c', 'python manage.py start_telegram_polling'] | ||
- notExists: | ||
path: spec.template.spec.containers[0].resources | ||
- matchSnapshot: | ||
path: spec.template.spec.containers[0].env |
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.
nit what about using a matchSnapshot
assertion? That should assert the same thing as these, without the need to explicitly type all of this out.
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.
changed to use matchSnapshot
for containers. I don't want to use snapshot for spec
as in that case snapshot will be too big. I think very big snapshots are difficult to read and to understand, if you need for some reason.
# Conflicts: # CHANGELOG.md # helm/oncall/tests/extra_env_test.yaml # helm/oncall/tests/mysql_env_test.yaml # helm/oncall/tests/mysql_password_env_test.yaml # helm/oncall/tests/postgres_env_test.yaml # helm/oncall/tests/postgres_password_env_test.yaml # helm/oncall/tests/telegram_env_test.yaml
What this PR does
Runs Telegram long polling to get updates.
It's enabled by setting
FEATURE_TELEGRAM_LONG_POLLING_ENABLED=True
. That will disable webhook and run separate deployment for telegram long polling.Telegram long polling is not very HA mode, but it does not need to expose webhook url to internet and simplifies telegram integration.
Which issue(s) this PR fixes
closes #561
Checklist
pr:no public docs
PR label added if not required)CHANGELOG.md
updated (orpr:no changelog
PR label added if not required)