Refactor client and fix tests #1513
Conversation
korgan00
requested review from
paaragon,
IceKhan13,
Tansito,
akihikokuroda and
psschwei
| time.sleep(1) | ||
| while time.time() < must_finish and not client: | ||
| try: | ||
| client = JobSubmissionClient(connection_url) |
There was a problem hiding this comment.
Trying to know if Ray server started. I didn't found a better way but I'm sure it is. For the moment, this works.
| @@ -40,9 +46,10 @@ def test_save_result(self): | |||
| ) | |||
| self.assertTrue(result) | |||
|
|
|||
| @patch("requests.get", Mock(return_value=ResponseMock())) | |||
There was a problem hiding this comment.
prevent token validation to fail.
|
I have to review the documentation. |
|
I converted it to draft @korgan00 just to let the team knows that we want a review on this but we need to fix tests yet |
Tansito
requested review from
a team
and removed request for
paaragon,
IceKhan13,
Tansito,
akihikokuroda and
psschwei
|
Also @korgan00 , most of the problems in the tests are related with the changes in the API. I can help you tomorrow looking at them but with some of the changes that I'm requesting you we should be able to solve some of them. |
|
Per offline conversations we also talked about changing |
|
Since file management is a Serveless feature that is not available in Ray or Local, should we remove it from the Base? This way we will reduce the |
korgan00
force-pushed
the
client-refactor
branch
from
2140199 to
a3c0809
Compare
|
Moving PR to review 🎉 |
| @@ -29,6 +27,7 @@ jobs: | |||
| shell: bash | |||
| run: | | |||
| cd tests/basic | |||
| rm ./06_function.py | |||
There was a problem hiding this comment.
These changes were necessary. Basically currently 06_function doesn't work in this test with the current docker-compose configuration and it was generating problems in the execution of the test.
|
Since the client and the job files has huge changes, they are not shown by default in the changes preview.You have to load them intentionally. I put the deprecation redirections directly in the base file because it is better to maintain than spread and repeat the code in every client implementation. |
There was a problem hiding this comment.
Solved my questions and concerns with @korgan00 on a call. From my part everything looks good but @IceKhan13 if you can double check I would appreciate it 👍
korgan00
force-pushed
the
client-refactor
branch
from
45c24bb to
9ae1121
Compare
Summary
Objectives:
BaseJobClientand migrate it toBaseClientRayJobClientand migrate it toRayClientLocalJobClientand migrate it toLocalClientGatewayJobClientand migrate it toServerlessClientJobrename the variable to use theclientinstead ofjob_client.QiskitFunctionsrename the variable to use theclientinstead ofjob_client.Details and comments
Additionally:
BaseClientimplementations (Ray, Local and Serverless) to their own files.BaseClientnow is officially an abstract class (ABSand@abstractmethod).ComputedResourcebecause it wasn't used correctly.