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

RFC: Job based API #1946

Closed
pvizeli opened this issue Aug 18, 2020 · 6 comments
Closed

RFC: Job based API #1946

pvizeli opened this issue Aug 18, 2020 · 6 comments

Comments

@pvizeli
Copy link
Member

pvizeli commented Aug 18, 2020

Context

Currently, each API call can end up with using process_lock to guard an object against new action while a current action is running.

Decision

We create a job monitor that replaces process_lock but works similarly and using https://docs.python.org/3/library/contextvars.html which makes it possible that we can call other function of this object without being deadlock or need to guard this.

The API works like before, but for some calls, it creates tasks/jobs and a new API to manage/show these jobs or getting details like progress. The job monitor is allowing us to use the core event bus to inform the UI about start/stop or progress on this job.

MyXy(JobGroup):

    job_group = "xy"

   @Job(lock=True, wait=False, name="Optional name if not method")
   def xy():

And generate /xy/update which is set on the first context and gets inherit to child jobs. So the frontend can get the correct root job. We can also lock future access over this JobGroup. The question is, how we deal with sub progress etc.

API:

job id task parent progress
/supervisor/update 13kj123lk1j2 Supervisor.update null null
jkfjsdwlf23423 DockerSupervisor.install 13kj123lk1j2 20%

Future

We can use this with logging to attach the right error message from deep back over the API to the UI.

@sanyatuning
Copy link

@pvizeli I realy want to help implement this but I need some hint.
What the new API should look like?
Here the current usage of process_lock.

    @process_lock
    def install(self, tag: str, image: Optional[str] = None, latest: bool = False):
        """Pull docker image."""
        return self.sys_run_in_executor(self._install, tag, image, latest)

@sanyatuning
Copy link

I started something in #1805.

@pvizeli
Copy link
Member Author

pvizeli commented Oct 20, 2020

Please make a new PR. Need a lot more code. Like an API and job manager as coresys object. The event is just a small part of this construct and replace also all locks on code. With test, that going into a 1-2 k line PR.

@sanyatuning
Copy link

Did you hear about MVP or KISS? :)

@pvizeli
Copy link
Member Author

pvizeli commented Dec 16, 2021

For history: #2348

@agners
Copy link
Member

agners commented Sep 6, 2023

Implemented by #4514.

@agners agners closed this as completed Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants