Implement serialization protocol in SwaggerClient class?

dominiquesydow commented 3 years ago

Issue

Using bravado's SwaggerClient in a multiprocessing protocol causes a RecursionError.

Example

Code for minimal example:

from multiprocessing import Pool

from bravado.client import SwaggerClient

API_DEFINITIONS = "https://klifs.net/swagger/swagger.json"
CLIENT = SwaggerClient.from_url(API_DEFINITIONS, config={"validate_responses": False})

pool = Pool(processes=2)
type_list = pool.map(type, [CLIENT, CLIENT])
print(type_list)
pool.close()
pool.join()

Output:

Process ForkPoolWorker-2:
Traceback (most recent call last):
  File "/home/dominique/.local/miniconda/envs/kissim/lib/python3.8/multiprocessing/process.py", line 315, in _bootstrap
    self.run()
  File "/home/dominique/.local/miniconda/envs/kissim/lib/python3.8/multiprocessing/process.py", line 108, in run
    self._target(*self._args, **self._kwargs)
  File "/home/dominique/.local/miniconda/envs/kissim/lib/python3.8/multiprocessing/pool.py", line 114, in worker
    task = get()
  File "/home/dominique/.local/miniconda/envs/kissim/lib/python3.8/multiprocessing/queues.py", line 358, in get
    return _ForkingPickler.loads(res)
  File "/home/dominique/.local/miniconda/envs/kissim/lib/python3.8/site-packages/bravado/client.py", line 170, in __getattr__
    return self._get_resource(item)    
return self._get_resource(item)
  File "/home/dominique/.local/miniconda/envs/kissim/lib/python3.8/site-packages/bravado/client.py", line 148, in _get_resource
    resource = self.swagger_spec.resources.get(item)

  # Repeating the line 170 and 148 errors over and over again

RecursionError: maximum recursion depth exceeded

Potential solution

Add serialization protocol in SwaggerClient by defining __getstate__ and __setstate.

@jaimergp managed to solve the problem in our opencadd package by implementing such a serialization protocol in a SerializationSwaggerClient subclass. Maybe something like this could be added directly to the SwaggerClient class?

macisamuele commented 3 years ago

@dominiquesydow thanks for reporting the issue. It would be great if you could open a PR yourself especially considering that you have found a way to address the issue.

Having such change would allow us to pass the SwaggerClient into a process Pool but more in general would allow us to pickle/unpickle the client (which might time consuming for big swagger specs)

dominiquesydow commented 3 years ago

Great, happy to do that within the next days!

Yelp / bravado