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

Add new Event type stateless #352

Merged
merged 5 commits into from
Mar 4, 2022
Merged

Conversation

stveit
Copy link
Contributor

@stveit stveit commented Mar 1, 2022

Fix for #346

Adds new event type called STATELESS. This is used instead of INCIDENT_START for the initial event when a stateless incident is created. Stateful incident will still start with a ÌNCIDENT_START event.

Adds a small test that checks if a stateless event is created when a new stateless incidents is created.

Adds migration files for adding STATELESStype and update old entries such that INCIDENT_START events that belong to stateless incidents get switched to STATELESS.

@hmpf hmpf self-requested a review March 3, 2022 08:31
Copy link
Contributor

@hmpf hmpf left a comment

Choose a reason for hiding this comment

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

Improve the migrations and see other comments.

Also, first letter upper case in commit-messages please. Hm, sounds like a candidate for a pre-commit hook!

Comment on lines 6 to 22
def update_event_type(apps, schema_editor):
# Change INCIDENT_START events for stateless incidents to STATELESS
Event = apps.get_model('argus_incident', 'Event')
for event in Event.objects.all():
if event.type == "STA" and event.incident.end_time == None:
event.type="LES"
event.save()

class Migration(migrations.Migration):

dependencies = [
('argus_incident', '0004_alter_event_type'),
]

operations = [
migrations.RunPython(update_event_type),
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this into the previous file (the function and append the RunPython to the end of the operations list.

Also, add a reversal function (second arg to RunPython).

src/argus/incident/models.py Show resolved Hide resolved
description=f"Incident #{source_incident_id} created for testing",
)
event_stateless = incident.events.filter(type=Event.Type.STATELESS)
self.assertEqual(1, len(event_stateless))
Copy link
Contributor

Choose a reason for hiding this comment

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

event_stateless is a queryset, so do event_stateless.count() instead of len(). This makes postgres do the count, not python.

@hmpf
Copy link
Contributor

hmpf commented Mar 3, 2022

Oh, and you're working from an outdated master-branch, so there might be merge-shenaningans.

stveit added 5 commits March 3, 2022 15:49
Instead of first event being INCIDIENT_START, now stateless
events use the STATELESS event type. Stateful incididents still
get an INCIDENT_START event
@stveit stveit force-pushed the event_type_stateless branch from 009d823 to d6c404a Compare March 3, 2022 15:09
@stveit
Copy link
Contributor Author

stveit commented Mar 3, 2022

Added reverse function for migration.
Put all migration in one file.
Reworded commits.
len(queryset) -> queryset.count().
Rebased on master.
Contemplated if there were any three letter combination better than LES.

Comment on lines +14 to +20
def remove_stateless_event_type(apps, schema_editor):
# undo changes made by add_stateless_event_type
Event = apps.get_model('argus_incident', 'Event')
for event in Event.objects.all():
if event.type == "LES":
event.type = "STA"
event.save()
Copy link
Contributor

@hmpf hmpf Mar 4, 2022

Choose a reason for hiding this comment

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

Chef's kiss! Mwah!

@hmpf hmpf self-requested a review March 4, 2022 08:13
@hmpf
Copy link
Contributor

hmpf commented Mar 4, 2022

Contemplated if there were any three letter combination better than LES.

I don't think there is, and I think it is three letters because that's the length of "END". The sole purpose of saving a string here instead of a key to a data table is for human convenience, and it isn't that much more expensive to have a much longer string, and not even demand the same length for each.

@stveit stveit merged commit ce63cb6 into Uninett:master Mar 4, 2022
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.

2 participants