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

Some improvements to Dockerfile + container deployment? #61

Open
hmacdope opened this issue Jul 15, 2024 · 6 comments · May be fixed by #62
Open

Some improvements to Dockerfile + container deployment? #61

hmacdope opened this issue Jul 15, 2024 · 6 comments · May be fixed by #62

Comments

@hmacdope
Copy link

Hi all,

Thanks you for the wonderful work! Very needed and fun to work with so far.

I have two small suggestions

  1. The current Dockerfile is not working (at least for me) and only contains rdkit and python. A faster solving, more lightweight container with a full environment could be advantageous (mamba docker images are great for this).
  2. It would be fantastic to have deployment of an already built container such that larger applications can just include this in a docker-compose setup. This can be done in a github workflow. I have made a demo in my fork of the repo. You can see the built package here: https://github.com/hmacdope/lightweight-registration/pkgs/container/lightweight-registration that you can try with docker run -it ghcr.io/hmacdope/lightweight-registration:main.

But of course you can decide which if any of these you would like to accept. I will open a PR and we can discuss there?

Cheers and thanks

Hugo

@greglandrum
Copy link
Contributor

Hi Hugo,

Thanks for the contribution.

The current Dockerfile is really only there to be used in the CI tests for lwreg

Would it perhaps make more sense to create a second one that is actually intended for deployment?

@hmacdope
Copy link
Author

Sounds very reasonable, where would you like it to go? devtools/deployment/ is normally what I use for stuff like this, but up to you.

@greglandrum
Copy link
Contributor

I think maybe just deployment/, but I don't have a strong opinion.
@brje01, what do you think?

@hmacdope
Copy link
Author

hmacdope commented Aug 5, 2024

Any further thoughts here? 😸

@greglandrum
Copy link
Contributor

Any further thoughts here? 😸

I think it's ok to take silence for agreement. :-)
Let's use just deployment/

@hmacdope
Copy link
Author

Sorry took me a while to circle back to this. I have pushed the relevant changes in my PR #62.

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 a pull request may close this issue.

2 participants