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

feature: add get_schedule for asset schedules #1086

Open
wants to merge 2 commits into
base: feature/api/endpoint-for-scheduling-asset
Choose a base branch
from

Conversation

Flix6x
Copy link
Contributor

@Flix6x Flix6x commented Jun 5, 2024

Description

  • Single endpoint to fetch the schedules of multiple devices as a single schedule.
  • We could still decide to also return a list of scheduling jobs, but when we move to a single job (for running a single scheduler for multiple sensors instead of multiple jobs, each running a scheduler for one sensor) such a list wouldn't exist anymore.

Look & Feel

{
    "schedule": [
        {
            "sensor": 1,
            "values": [
                2.15,
                3,
                2
            ],
            "start": "2015-06-02T10:00:00+00:00",
            "duration": "PT45M",
            "unit": "MW"
        },
        {
            "sensor": 2,
            "values": [
                2.15,
                3,
                2
            ],
            "start": "2015-06-02T10:00:00+00:00",
            "duration": "PT45M",
            "unit": "MW"
        }
    ]
}

How to test

  • PR includes test that covers new endpoint.

Further Improvements

  • Add test for 2 sensors each with a different resolution.
  • Requested duration should match all sensor resolutions.

Related Items

Follow-up to #1065.

Copy link
Contributor

@victorgarcia98 victorgarcia98 left a comment

Choose a reason for hiding this comment

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

Awesome!

Basically pointing some low effort improvements for the test.

url_for(
"SensorAPI:get_schedule", id=sensor.id, uuid=scheduling_job.id
), # todo: use (last?) job_id from trigger response
url_for("AssetAPI:get_schedule", id=sensor.id, uuid=job_id),
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we can try to get the schedules right before letting the worker take the jobs (Line 85). Also, before the jobs are enqueued to test the error of a non-existent job.

@@ -312,6 +312,9 @@ def create_sequential_scheduling_job(
connection=current_app.queues["scheduling"].connection,
)

# Stand-in for MultiStorageScheduler
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

I wonder if we should reflect that the output of the schedules used a sequential approach..

consumption_schedule = consumption_schedule[
start : start + duration - resolution
]
sensor_schedule_response = dict(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this one include the scheduler_info as well? That way we can detect which schedules were fallback and which not.

)

overall_schedule_response = []
for child_job in job.fetch_dependencies():
Copy link
Contributor

@victorgarcia98 victorgarcia98 Jun 5, 2024

Choose a reason for hiding this comment

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

It looks that there's is quite a large overlap between this logic and the one found in the sensor get_schedule endpoint. I'd say to keep it like that for now given that this endpoint might still get some changes when we introduce the non-sequential algorithm.

@victorgarcia98
Copy link
Contributor

On the side, I wonder about the case where we do a centralized planning and we want the assets to get their schedules. Maybe in this case the devices can query the schedule "blindly" with certain frequency.

@Flix6x
Copy link
Contributor Author

Flix6x commented Jun 5, 2024

On the side, I wonder about the case where we do a centralized planning and we want the assets to get their schedules. Maybe in this case the devices can query the schedule "blindly" with certain frequency.

If we do a centralized planning, we plan them all at once, so either none of them are done or all of them are done, right?

@victorgarcia98
Copy link
Contributor

On the side, I wonder about the case where we do a centralized planning and we want the assets to get their schedules. Maybe in this case the devices can query the schedule "blindly" with certain frequency.

If we do a centralized planning, we plan them all at once, so either none of them are done or all of them are done, right?

Yes but the planning needs to happen from only one point. I'm thinking in the case where we do trigger the plan on our side (for example via a CLI on a cronjob) and the devices request their schedules. This is one idea we had for TUNES.

@Flix6x
Copy link
Contributor Author

Flix6x commented Jun 5, 2024

Oh yes, you are absolutely right. Thanks for jolting my memory. It should be made possible for the individual devices to get their own schedule using the job ID of the collective scheduling job. I can make that happen, probably by adding a few lines on the original get_schedule endpoint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants