Flask to klein no agents #450
Conversation
This is necessary to be able to circumvent joblib issue 180 joblib/joblib#180 - Moved the pool of workers from the server into the datarouter - Make use of concurrent.future ProcessPoolExecutor - May have broken an insane amount of code - Need to figure out how to pickle the result's objects from the workers processes
Conflicts: rasa_nlu/model.py
Conflicts: rasa_nlu/components.py
|
As multiprocessing introduced a lot of hacky code, I gave another try at |
| __version__ = None # Avoids IDE errors, but actual version is read from version.py | ||
| exec(open('rasa_nlu/version.py').read()) | ||
| __version__ = None # Avoids IDE errors, but actual version is read from version.py | ||
| exec (open('rasa_nlu/version.py').read()) | ||
|
|
||
| tests_requires = [ |
There was a problem hiding this comment.
This needs to be edited, doesn't it?
There was a problem hiding this comment.
All packages listed in requirements.txt and dev-requirements.txt should also be listed here, shouldn't they?
There was a problem hiding this comment.
so all requirements in requirements.txt should show up here, yes. dev_requirements.txt are optional
| self.config = config | ||
| self.responses = DataRouter._create_query_logger(config['response_log']) | ||
| self._train_procs = [] | ||
| self._trainings_queued = 0 |
There was a problem hiding this comment.
I know the low-level training process tracking was replaced by this simple counter. But, in my opinion, as the processes are now managed by the ProcessPool object, they are too low-level to be accessed and managed directly. This situation could be improved a lot by finishing the transition to a higher-level endpoints tracking (see my other PR).
rasa_nlu/server.py
Outdated
| @app.route("/train", methods=['POST']) | ||
| @requires_auth | ||
| def train(self, request): | ||
| def errback(f): |
There was a problem hiding this comment.
Here we could take advantage of the server's async nature by only answering the client once the training is done or failed (the server can still handle other clients requests).
There was a problem hiding this comment.
that sounds great, let's do that 👍
There was a problem hiding this comment.
but make sure to mention that in the changelog and the migration file (as anyone calling that api will now need to wait till it is finished training)
|
Coverage looks good, nice 👍 I think we are very close, exiting 🎉 Final steps:
Is there anything that needs to be addressed in the documentation? I don't think so, right? |
|
As for the doc, one cannot longer make use of gunicorn + wsgi as the asynchronous processing is not part of the WSGI spec. |
|
ok let's remove gunicorn + wsgi then. Nevermind, |
- A request to the `/train` endpoint will now wait for the server answer - CORS checking has now its own decorator - Introduced another codepath for testing the training - Some minor test fixes
…sa_nlu into flask_to_klein_no_agents
|
I think it's ready for merge. |
|
🎉 great work 👍 |
|
Thanks. Let's see how many new issues will be created following that. |
Proposed changes:
Rasa logic should work with agents (endpoints) instead of models.(Moved to Agents endpoints #449)Status:
Issues introduced by this change:
Multiprocessing backed parallel loops cannot be nested below threads, setting n_jobs=1 joblib/joblib#180Original PR: #426