-
Notifications
You must be signed in to change notification settings - Fork 38
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
API endpoint for scheduling asset #1065
base: main
Are you sure you want to change the base?
Changes from 1 commit
e2fced5
bfdd94f
5795e1b
2e4cfe0
2e28ed9
f3711de
305d76d
f6e8075
4bf91dc
0917896
753b411
ba90dd1
af4debf
49a1a12
8a3e112
25d1180
21ac09a
b4ad01a
5b4c30f
484a022
8374a9c
4621ef6
ff22fea
be51f1b
061e1f3
6eb122b
b204e4b
22834ca
dc07b11
b583c89
d04bed0
5fd2f63
ee5e19e
c2bf1af
529baa2
1665974
8678a0d
f42ceaf
7b5b3ce
e9e6ec9
378b0a7
754502a
e1e1019
4f86bb1
de811ec
175cd18
ffe82b1
45592c7
99bd8da
44cfd0c
ee08883
a9ddce8
f340a95
4c6c52a
b328921
b40d40e
b7961f4
429d97f
8423522
c60f5cf
8ba79aa
9f5eb6a
71da5c4
07b44b6
a25ab4d
5f9a4dc
1f26fa2
8b0c0d0
edf38a8
05064ae
c7e58cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -288,7 +288,7 @@ def create_sequential_scheduling_job( | |
scheduler_specs=scheduler_specs, | ||
requeue=requeue, | ||
job_id=job_id, | ||
enqueue=enqueue, | ||
enqueue=False, # we enqueue all jobs later in this method | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not too sure about my change here, but I believe I saw duplicate jobs in the queue otherwise. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are enqueuing them later altogether. We are not checking the function argument, though. This is something to be fixed. |
||
depends_on=previous_job, | ||
force_new_job_creation=force_new_job_creation, | ||
) | ||
|
@@ -323,10 +323,10 @@ def create_sequential_scheduling_job( | |
for job in jobs: | ||
current_app.queues["scheduling"].enqueue_job(job) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably move the enqueue here.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean doing something like |
||
current_app.job_cache.add( | ||
asset_or_sensor["id"], | ||
asset_or_sensor.id, | ||
job.id, | ||
queue="scheduling", | ||
asset_or_sensor_type=asset_or_sensor["class"].lower(), | ||
asset_or_sensor_type=str(type(asset_or_sensor)).lower(), | ||
) | ||
|
||
return jobs | ||
|
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.
Technically, I guess I now implemented this todo, by putting the last job ID (that of the 'done job') in the response. However, I recall we discussed returning a full list of job IDs, so that the API user can use the existing
get_schedule
endpoint to retrieve schedules for individual sensors. In that case, the next step would be to spec such a response.For example:
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.
True, returning a list of schedules would be the way to go. That way, we can reuse the endpoint to get the schedules.