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

GO-75 Set jobs priority #461

Merged
merged 21 commits into from
Nov 16, 2024
Merged

GO-75 Set jobs priority #461

merged 21 commits into from
Nov 16, 2024

Conversation

luciajanikova
Copy link
Member

Prvy nastrel pridania priorit k jobom. Uvazujem, ci takto hadzat priority k jednotlivym jobom alebo si radsej spravit queues podla priority. Prve priblizenie som zatial dala cez priority.
Pri initial UPVS importe sa mi podarilo posuvat prioritu cez vsetky joby, az na tento event:
https://github.com/slovensko-digital/govbox-pro/blob/main/app/lib/event_bus.rb#L36-L38, ktory sa vyvola po vytvoreni spravy. Tam neviem ako by sme to rozumne posunuli.

Initial UPVS import: nizka priorita (100)
Pravidelne stahovanie updates UPVS, FS formularov: najnizsia priorita (1000)
Userom vyvolany sync: najvyssia priorita (-1000)
Prevzatie spravy do vlastnych ruk: najvyssia priorita (-1000)

@jsuchal
Copy link
Member

jsuchal commented Jul 25, 2024

@luciajanikova moj nazor na priority jobov je nasledovny: Vzdy sa na to treba pozerat z pohladu SLA voci zakaznikovi.

Cize za mna:

  1. ak niekde kliknem a cakam, ze sa to udeje takmer okamzite (typicky odoslanie podania, zmena tagu/indexovanie...) tak to je typ "immediate".
  2. ak cakam, ze sa to udeje niekedy v dohladnej dobe, ale nie nutne okamzite, to je taky default - priklad - odoslanie hromadneho podania.
  3. ak je to vec, ktoru user nevidi a nezalezi ci to bude o hodinu ci dve neskor - typicky prvotny import - nizka priorita. sem by som dal asi aj rozne stahovania

Ciselne priority nemam uplne rad lebo v tom sa hned zamotas (uz len to ze ci nizke cislo alebo vysoke ide skor). Lepsie je to nazvat nejako rozumne: immediate, default, low_priority.

Druhy pohlad je, vyrabat queue na kazdy job - to ma vyhodu, ze pripadne vies nejake typy jobov vypnut, ale to u nas netreba.

# Conflicts:
#	app/jobs/govbox/process_message_job.rb
@@ -25,7 +25,7 @@ class Govbox::Message < ApplicationRecord

DELIVERY_NOTIFICATION_TAG = 'delivery_notification'

def self.create_message_with_thread!(govbox_message)
def self.create_message_with_thread!(govbox_message, priority: :default)
Copy link
Member Author

Choose a reason for hiding this comment

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

Tu sme si @jsuchal povedali, nech si radsej posielame kontext do tejto metody ako prioritu (a vo vnutri metody to prelozime na prioritu). Vidim vsak v tom trochu problem, lebo kontext moze byt rozny. Inicialny import (nizka priorita), stahovanie spravy do vlast. ruk po prevzati dorucenky (vysoka priorita) alebo ziadny kontext (default priorita).
A tento kontext nemame velmi ako v ProcessMessageJobe (ktory vola tuto metodu) poznat, museli by sme si ho tam posielat napriec viacerymi jobmi, aby vedel prebublat az sem.

# Conflicts:
#	test/fixtures/message_object_data.yml
#	test/fixtures/message_objects.yml
#	test/system/message_drafts_test.rb
Comment on lines -573 to +578
t.enum "color", enum_type: "color"
t.integer "tag_groups_count", default: 0, null: false
t.string "icon"
t.integer "tag_groups_count", default: 0, null: false
t.enum "color", enum_type: "color"
t.index "tenant_id, type, lower((name)::text)", name: "index_tags_on_tenant_id_and_type_and_lowercase_name", unique: true
t.index ["owner_id"], name: "index_tags_on_owner_id"
t.index ["tenant_id", "type"], name: "signings_tags", unique: true, where: "((type)::text = ANY (ARRAY[('SignatureRequestedTag'::character varying)::text, ('SignedTag'::character varying)::text]))"
t.index ["tenant_id", "type"], name: "signings_tags", unique: true, where: "((type)::text = ANY ((ARRAY['SignatureRequestedTag'::character varying, 'SignedTag'::character varying])::text[]))"
Copy link
Member Author

Choose a reason for hiding this comment

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

Ani tieto zmeny sa podla mna netykaju tohto PR.

Copy link
Member

Choose a reason for hiding this comment

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

Toto som recreate from scratch cize toto sa tam udialo. neviem preco

Copy link
Member Author

@luciajanikova luciajanikova left a comment

Choose a reason for hiding this comment

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

Dva komenty som hodila k veciam, ktore su tu podla mna navyse, inak by som skusila nasadit.

@luciajanikova luciajanikova merged commit 9172720 into main Nov 16, 2024
3 checks passed
@luciajanikova luciajanikova deleted the GO-75/jobs_priority branch November 16, 2024 16:37
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