Run refactorization

[Tansito]

Summary

Close #1240

This PR continues the work of #1267. The logic is pretty similar the biggest difference is that here we remove services references because we are moving all the logic to the serializers, finally.

Details and comments

• I removed exceptions too because they were not working as expected. I will continue this effort as part of Client + gateway: improve error handling  #1264

[akihikokuroda]

LGTM! Thanks!

[Tansito]

I'm going to wait for your merges @akihikokuroda and wait a bit for @psschwei or @IceKhan13 if they can take a look

[psschwei]

Looks like docker/kubernetes tests are both failing with the same error in the running_programs_using_decorators notebook:

quantum_serverless.exception.QuantumServerlessException: 
| Message: Http bad request.
| Code: 400
| Details: {"config":["This field is required."]}

Does the decorators code need an update?

[akihikokuroda]

I don't see the changes in client/quantum_serverless/core/job.py

[Tansito]

I found a problem with #1285 I'm fixing it now and put here the problem, @akihikokuroda

[Tansito]

Tests passed so here is what I found, @akihikokuroda . With run end-point if we set config the data that we generate in the client is the next one:

Configuration(workers=None, min_workers=None, max_workers=None, auto_scaling=False, python_version='py39')

{'title': 'my-first-pattern', 'entrypoint': 'basic-pattern.py', 'arguments': '{"test": "here it goes an argument"}', 'dependencies': '[]', 'config': {'workers': None, 'min_workers': None, 'max_workers': None, 'auto_scaling': False, 'python_version': 'py39'}}

Until here everything is fine with the changes from #1285

The problem comes in the gateway:

<QueryDict: {'title': ['my-first-pattern'], 'entrypoint': ['basic-pattern.py'], 'arguments': ['{"test": "here it goes an argument"}'], 'dependencies': ['[]'], 'config': ['workers', 'min_workers', 'max_workers', 'auto_scaling', 'python_version'], 'artifact': [<InMemoryUploadedFile: artifact.tar ()>]}>

I don't know why but Django transforms config into:

'config': ['workers', 'min_workers', 'max_workers', 'auto_scaling', 'python_version']

What it generates the error. We didn't find this problem basically because we don't have a test for run end-point with config and old logic was not taking into account config format (I will work in one test later). So I needed to restore the changes for run method from #1285 .

My question now is, with Content-type: multipart/form-data we cannot send JSON (I don't know if the problem is the content-type or the QueryDict). Should we send config as string in run_existing too to just not have different logic between these end-points, @akihikokuroda ?

[akihikokuroda]

I saw Django handled the Content-type and the config field without converting to the string. You may merge this PR as-is. I can try the #1285 later.

[Tansito]

Ok, I'm going to prepare a test for this anyway before merge and put on-hold #1290 until we clarify this. Thanks, Aki.

[psschwei]

With run end-point if we set config the data that we generate in the client is the next one:

Configuration(workers=None, min_workers=None, max_workers=None, auto_scaling=False, python_version='py39')

{'title': 'my-first-pattern', 'entrypoint': 'basic-pattern.py', 'arguments': '{"test": "here it goes an argument"}', 'dependencies': '[]', 'config': {'workers': None, 'min_workers': None, 'max_workers': None, 'auto_scaling': False, 'python_version': 'py39'}}

Is that configuration part of our code somewhere? (wasn't clear if it was or just an example you made up) If so, do we need to update the python version to py310?

[Tansito]

Is that configuration part of our code somewhere? (wasn't clear if it was or just an example you made up) If so, do we need to update the python version to py310?

Oh no, no @psschwei . They're just some examples that I'm doing to test things locally hehe.

[psschwei]

I have a question about names, but otherwise LGTM

[psschwei]

long-term thought: would it make sense to refactor the API to just use a single /run endpoint and distinguish within the call whether the program/function/pattern/job/building_block/element/quantum_magic has already been uploaded or not?

[Tansito]

Definitely I think it's something that we can propose. I had in mind to continue the improvements in the gateway from a code organization point of view: #1286 (comment)

I considered too that we could refactor /run end-point to /upload + /run_existing in the client or something like that. Feel free to open an issue or a discussion because I think it deserves the times the refactorization.

[psschwei]

this is maybe more for a later refactoring (change wasn't introduced in this PR, so let's not change now), but do we call non-v1 serializers here? if not, given that we're already in a v1/ directory the prefix is a little redundant

[Tansito]

Yep, I agree. It could be simplified.

[Tansito]

I fixed the problems. Added some tests and ensure that everything is working. I planned to merge at the end of the day if you don't see anything that I should change. Let me know 🙏