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

Docker set up to support reading checkpoint_file from config.json and documentation #38

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ntlex
Copy link

@ntlex ntlex commented Sep 23, 2021

Hi,

Created a PR to support passing the model's checkpoint_files in a docker container set up. Here's a sort description of the changes:

  1. Even though in label_studio_ml/default_configs/_wsgi.py.tmpl there is support for reading the checkpoint_file through config.json (see method get_kwargs_from_config()) the json import is missing as well as making the kwargs available to the uWSGI app.
  2. Added config.json in the default_configs
  3. Updated Dockerfile to copy the config.json as well as the checkpoint_file that should be placed in ./models/
  4. Updated README.md with the instructions on using the backend with docker, including considering the cloud storage credentials and checkpoint file exposure

…json, resolves missing configuration in _wsgi.py.tml and updates README.md for docker set up instructions
@@ -15,7 +15,9 @@ COPY supervisord.conf /etc/supervisor/conf.d/

WORKDIR /app

COPY /models/*.pt /app/data/models/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ntlex What if /models doesn't exist? Looks like it's a very user/system specific folder.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, I could remove this line since it is elaborated in the the README.md to ensure that the user either has to volumize or copy the model dir in the docker application.

Copy link
Member

@makseq makseq Sep 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe it's better to check existence of this folder? and then COPY it

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I misunderstood your initial comment about the user specific directory. I have corrected the directory's path to ./models/*.pt on the latest commit. Let me know if this is what you had in mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants