Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python-wrapper: Use debug flag to provide useful information #309

Closed
epa095 opened this issue Nov 22, 2018 · 4 comments · Fixed by #488
Closed

Python-wrapper: Use debug flag to provide useful information #309

epa095 opened this issue Nov 22, 2018 · 4 comments · Fixed by #488
Assignees
Projects
Milestone

Comments

@epa095
Copy link

epa095 commented Nov 22, 2018

The python model microservice accepts a debug-flag, but it does not use it for anything. It seems reasonable for it to use it for at least two things:

  • Pass it along to the flask server, so it can be in debug/non-debug mode, and
  • Provide different level of logging. For the latter I recommend using the logging module in Python instead of "print" statements scattered around in the code:
    • On the top of the class get the logger with logger = logging.getLogger(__name__)
    • Around in the code log to different levels, using e.g. logger.debug or logger.info
    • Set the loglevel of the logger based on the debug toggle.
@ukclivecox
Copy link
Contributor

These changes make sense. We are updating the python code to create a Seldon module for easier testing (#306) and usage and we can add these changes into that. @jklaise

@ukclivecox ukclivecox added this to To do in 0.2.6 via automation Jan 27, 2019
@ukclivecox ukclivecox added this to the 0.2.x milestone Jan 27, 2019
@ukclivecox ukclivecox moved this from To do to In progress in 0.2.6 Feb 17, 2019
@ukclivecox ukclivecox removed this from In progress in 0.2.6 Feb 18, 2019
@ukclivecox ukclivecox added this to In progress in 0.2.7 Feb 21, 2019
@ukclivecox ukclivecox assigned ukclivecox and unassigned jklaise Feb 21, 2019
@ukclivecox ukclivecox moved this from In progress to Done in 0.2.7 Apr 4, 2019
@masonlr
Copy link
Contributor

masonlr commented Jul 19, 2019

@cliveseldon #306 was merged, but did hot-reloading make it into the seldon_core.microservice module?

Hot-reloading would be useful for iterative development. A use case would be to mount local files into a model container using something like:

docker run -it -p 5000:5000 -v ${PWD}:/microservice name:version /bin/bash

then run a debug mode from within the container (although with a --debug flag, or a DEBUG environment variable, etc.)

seldon-core-microservice Model REST --service-type MODEL --debug 

and then be able to make local file changes and see the model hot-reload. The change would be to support passing debug variable state through to (https://github.com/SeldonIO/seldon-core/blob/master/python/seldon_core/microservice.py#L224):

 app.run(host='0.0.0.0', port=port, debug=True)

A hot-reload option might also be useful for accelerating k8s iterative development if using dev tools like skaffold filesync (https://skaffold.dev/docs/how-tos/filesync/).

@ukclivecox
Copy link
Contributor

This sounds useful. You happy to do a PR for adding that to debug mode?

@masonlr
Copy link
Contributor

masonlr commented Jul 19, 2019

Yes can do. Do you have a preference for the interface? i.e. --debug flag or specific env variable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
0.2.7
  
Done
Development

Successfully merging a pull request may close this issue.

4 participants