Function and jobs refactor - resolve circular dependencies #1517
Conversation
# Conflicts: # client/qiskit_serverless/core/client.py # client/qiskit_serverless/core/clients/local_client.py # client/qiskit_serverless/core/clients/ray_client.py # client/qiskit_serverless/core/clients/serverless_client.py
# Conflicts: # client/qiskit_serverless/core/client.py # client/qiskit_serverless/core/clients/local_client.py # client/qiskit_serverless/core/clients/ray_client.py # client/qiskit_serverless/core/clients/serverless_client.py # client/qiskit_serverless/core/files.py # client/qiskit_serverless/core/function.py # client/qiskit_serverless/core/job.py
| @abstractmethod | ||
| def run( | ||
| self, | ||
| program: Union[QiskitFunction, str], | ||
| arguments: Optional[Dict[str, Any]] = None, | ||
| config: Optional[Configuration] = None, | ||
| ) -> Job: | ||
| """Execute a program as a async job. | ||
|
|
||
| Example: | ||
| >>> serverless = QiskitServerless() | ||
| >>> program = QiskitFunction( | ||
| >>> "job.py", | ||
| >>> arguments={"arg1": "val1"}, | ||
| >>> dependencies=["requests"] | ||
| >>> ) | ||
| >>> job = serverless.run(program) | ||
| >>> # <Job | ...> | ||
|
|
||
| Args: | ||
| arguments: arguments to run program with | ||
| program: Program object | ||
|
|
||
| Returns: | ||
| Job | ||
| """ | ||
|
|
||
| @abstractmethod | ||
| def status(self, job_id: str) -> str: | ||
| """Check status.""" | ||
|
|
||
| @abstractmethod | ||
| def stop( | ||
| self, job_id: str, service: Optional[QiskitRuntimeService] = None | ||
| ) -> Union[str, bool]: | ||
| """Stops job/program.""" | ||
|
|
||
| @abstractmethod | ||
| def result(self, job_id: str) -> Any: | ||
| """Return results.""" | ||
|
|
||
| @abstractmethod | ||
| def logs(self, job_id: str) -> str: | ||
| """Return logs.""" | ||
|
|
||
| @abstractmethod | ||
| def filtered_logs(self, job_id: str, **kwargs) -> str: | ||
| """Return filtered logs.""" | ||
|
|
There was a problem hiding this comment.
This methods are defined in new RunService and JobClient abstract classes.
client/qiskit_serverless/core/job.py
Outdated
| @@ -76,13 +77,44 @@ class Configuration: # pylint: disable=too-many-instance-attributes | |||
| auto_scaling: Optional[bool] = False | |||
|
|
|||
|
|
|||
| class JobClient(ABC): | |||
There was a problem hiding this comment.
As we commented through a call JobClient I would like to rename it to JobService to match it with the current RunService.
Apart from that I saw everything good, really good job here @korgan00
|
@IceKhan13 if you can take a look to this PR too and give some feedback to @korgan00 it would be nice! |
There was a problem hiding this comment.
Very good job @korgan00 😄
I have a question regarding function.py and job.py.
job.pycontains three classes:Configuration,JobServiceandJobfunction.pycontains another three classes:QiskitFunction,RunServiceandRunnableQiskitFunction.
Would it make sense to have some of these classes in a separated file? I was thinking that maybe RunService and JobService could be in another file
| version: version of a program | ||
| """ | ||
|
|
||
| _run_service: RunService = None |
There was a problem hiding this comment.
| _run_service: RunService = None | |
| _run_service: Optional[RunService] = None |
There was a problem hiding this comment.
In this case i think it shouldnt be optional because, to be runnable it needs an associated run service. If run_service is None, then it is just a QiskitFunction
Since they are very coupled to that files, I added it to the same file, but im ok with splitting it. What do you think @IceKhan13 , @Tansito ? |
|
I think by now having them in the same file is good enough, as we commented we are not expecting to re use them in other places in the near future, @korgan00 👍 |
Summary
Depends on #1513
Details and comments
To resolve the circular dependencies,
QiskitFunctionandJobdefines an interface (pure abstract class in python) with their needs:RunServiceandJobService. The oldclient: Anyvariable will be replaced with_run_service: RunServiceand_job_service: JobService. TheBaseClientshould implement this interfaces.We are achieving multiple objectives:
QiskitFunctionandJobdoesn't need to importBaseClient, the imports are in theBaseClientside._run_serviceand_job_servicevariables now can be resolved by the IDE and offer autocompletion and type-checking of the linter and code-quality tools.QiskitFunctionandJobare decoupled.Additionally:
QiskitFunctionhad therunmethod that raised an exception by default if you create it and it worked when it is retrieved from the client.The
QiskitFunctionno longer has arunmethod because it doesn't work without a client.RunnableQiskitFunctioninherits from it, implements therunand receives the client as a not optionalrun_service. This is less error prone.Client's
upload,functionandfunctionsnow returnRunnableQiskitFunctionby default.