-
Notifications
You must be signed in to change notification settings - Fork 59
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 run-on to Boefje Setup page #4061
Conversation
…minvws/nl-kat-coordination into feature/boefje-variants-run-on-
…e" for future extensions - Fix the unit and integration tests - Fix scheduler definition - Fix scheduler tests
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.
scheduler changes lgtm!
- Allow the API to post and get a list of strings but save them as a proper PSQL Enum - Test RunOn conversion and integration tests for the API
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, minimal remarks
CREATE_UPDATE = "create_update" | ||
|
||
@classmethod | ||
def from_run_ons(cls, run_ons: list[RunOn] | None): |
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.
def from_run_ons(cls, run_ons: list[RunOn] | None): | |
def from_run_ons(cls, run_ons: list[RunOn] | None) -> RunOn | None: |
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.
@madelondohmen This should be Self
from typing_extensions (probably) it should actually return RunOnDB | None
which won't work because it references itself.
def from_run_ons(cls, run_ons: list[RunOn] | None): | |
def from_run_ons(cls, run_ons: list[RunOn] | None) -> Self | None: |
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.
@Donnype is right. "RunOn | None" won't work since it returns a RunOnDB. But Self doesn't work as well. So I suggest to leave it as it was.
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.
typing.Self
is introduced in Python 3.11, but we still use 3.10 for development. Therefore:
def from_run_ons(cls, run_ons: list[RunOn] | None) -> "RunOnDB" | None:
@@ -232,3 +254,18 @@ def create_boefje_with_form_data(form_data, plugin_id: str, created: str | None) | |||
oci_image=form_data["oci_image"], | |||
oci_arguments=arguments, | |||
) | |||
|
|||
|
|||
def get_interval_minutes(interval_number, interval_frequency) -> int | None: |
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.
def get_interval_minutes(interval_number, interval_frequency) -> int | None: | |
def get_interval_minutes(interval_number: int, interval_frequency: str) -> int | None: |
- It would be nicer to throw a
ValueError
on invalid parameter (e.g. unknowninterval_frequency
argument) - You can use
Literal["minutes", "hours", ...]
to type hint this literal of choices
Co-authored-by: Donny Peeters <[email protected]>
Co-authored-by: ammar92 <[email protected]>
I've made the Javascript more general, and reusable for other variants of the same logic. |
Oops, copy paste error. Is now fixed. |
|
Changes
This PR adds the possibility for a user to select "Run On" to the Boefje (variant).
The user can choose one out of two options for when the Boefje should run:
If the user selects one of the options, the other option should be empty/None.
It is also possible for the user to not select any of the options. In that case, the default system scan frequency will be used.
Issue link
Closes #3929
Demo
Opname.2025-02-04.111401.mp4
QA notes
Check if all of this works:
Also check these two flows:
Code Checklist
.env
changes files if required and changed the.env-dist
accordingly.Checklist for code reviewers:
Copy-paste the checklist from the docs/source/templates folder into your comment.
Checklist for QA:
Copy-paste the checklist from the docs/source/templates folder into your comment.