Feature/ai builder connector #408
Conversation
docker-compose.yaml
Outdated
| volumes: | ||
| - ./src:/app | ||
| - ${DATA_PATH}/connectors:/opt/connectors/data | ||
| - ./connectors/aibuilder/:/opt/connectors/script |
There was a problem hiding this comment.
Please include the default scripts directly in the image, optionally provide mounted scripts for live updates in the docker-compose.dev.yaml. This makes it so that when running without USE_LOCAL_DEV, you do not accidentally include local changes (provided images were built with a clean slate). Mounting the data directory (for writing logs) can stay, of course. See for example the zenodo connector.
| response = requests.get(url, timeout=REQUEST_TIMEOUT) | ||
| if not response.ok: | ||
| status_code = response.status_code | ||
| msg = response.json()["error"]["message"] |
There was a problem hiding this comment.
This errors when the content of the response is not JSON. I found that for the 401: UNAUTHORIZED response, the server does not return JSON, so the connector would crash.
There was a problem hiding this comment.
I added a try - except block returning a record error (it is maybe too generic error for that, but I think it is the proper way in this context to return it)
There was a problem hiding this comment.
The problem is not with the request itself (though I guess there can also be errors there). The request is successful but it will have status 401: UNAUTHORIZED and content b'authentication: null' or something similar. The latter is not JSON, so the response.json() call below will result in an error.
There was a problem hiding this comment.
Ah, ok. I took this behaviour from the openml connectors, so maybe we should revise them too. I am looking now at the zenodo one and I will change the approach to that one
|
I made all the modifications, also added an init method to be able of passing a non-empty TOKEN for the tests. |
|
I modified the code to handle the responses that are correct but not a 200, added an specific error message for the 401 error and made a new unitary test for it. Also removed the .env file from docker compose. |
Change
New connector for the AIBuilder platform
How to Test
There are unitary test created for it.
To test the actual functionality you need to add a valid API_TOKEN (from a user of the platform) to the 'override.env' file and use the 'aibuilder' profile when launching docker compose. Then you will see new ml_models added to the platform (you can search selecting the 'aibuilder' platform).
Checklist
The only thing I didn't do is to add an explanation on the README (in general I see that there are no such explanations for the use of other connectors, but maybe the TOKEN thing should be commented). I guess we can discuss this on the PR and decide if it needs some explanation.