Skip to content
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

fix: notifier state get's never persisted in database #380

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

roock
Copy link
Contributor

@roock roock commented Jun 2, 2022

  • every time the notification starts, it reports a corrupted state
    because the state never gets stored in the database.
    saving the state in the database using UPDATE sql only works
    when there is already one entry in the table, otherwise the query
    will run through with zero updated rows.
    so in the case when the state cannot be loaded, we properly create
    one record inside the notifier_state table

window_start)
# create a clean state in the datebase
cursor.execute('TRUNCATE `notifier_state`')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix Flake8 errors and make sure circleci tests are passing

src/oncall/notifier/reminder.py:54:52: W291 trailing whitespace

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src/oncall/notifier/reminder.py Outdated Show resolved Hide resolved
@roock roock force-pushed the rp_fix_notifier_state branch from 39152bd to 7eaa049 Compare June 3, 2022 11:03
- ever time the notification starts, it reports a corrupted state
  because the state never gets stored in the databse.
  saving the state in the database using `UPDATE` sql only works
  when there is already one entry in the table, otherwise the query
  will run through with zero updated rows.
  so in the case when the state cannot be loaded, we properly create
  one record inside the notifier_state table
@roock roock force-pushed the rp_fix_notifier_state branch from 7eaa049 to 2993b4a Compare June 3, 2022 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants