theochem / Selector

Methods for selecting diverse (molecular) database.
https://selector.qcdevs.org
GNU General Public License v3.0
22 stars 20 forks source link

Add Basic Interfaces and CI/CD for web interface #212

Open JackyZzZz opened 3 weeks ago

JackyZzZz commented 3 weeks ago

This PR introduces the basic interfaces of our package, including the intro and MaxMin page. Moreover, it established a complete CI/CD protocol to deploy the associated docker image to DockerHub and HuggingFace for deployment.

The docker image is deployed on DockerHub.

The application is live on HuggingFace.

There are a few things that should be checked before merging:

    • [ ] Create a DockerHub account for our organization for hosting the Docker image.
    • [ ] Create a HuggingFace account and space for our organzation to host the application.
    • [ ] Set up Github Secrets in the repo, including Dockerhub and HuggingFace credentials.

Here is a demo of this PR:

https://github.com/theochem/Selector/assets/110846941/5fa794ba-be86-4daf-92aa-7dedcd61240c

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.00%. Comparing base (d931154) to head (0dfb9f8). Report is 4 commits behind head on main.

:exclamation: Current head 0dfb9f8 differs from pull request most recent head 1b4d508

Please upload reports for the commit 1b4d508 to get more accurate results.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/theochem/Selector/pull/212/graphs/tree.svg?width=650&height=150&src=pr&token=0UJixrJfNJ&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=theochem)](https://app.codecov.io/gh/theochem/Selector/pull/212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=theochem) ```diff @@ Coverage Diff @@ ## main #212 +/- ## ======================================= Coverage 96.00% 96.00% ======================================= Files 9 9 Lines 975 975 ======================================= Hits 936 936 Misses 39 39 ```
JackyZzZz commented 2 weeks ago

@FanwangM I noticed the workflow failed during docker build stage. It looks like there were some problem after we bump the python version to 3.11. I fixed it with using the convention of 3.11 of pep517 and it was able to run with no problem. Could you try to run it again when you are available?

Also, I think it might be beneficial if you can set up the dockerhub and HuggingFace credential in the repo's secrets. Otherwise the workflow will not work because it depends on the secrets of the repo, which our official repo haven't set up yet.

JackyZzZz commented 2 weeks ago

There was also 2 places I would appreciate your opinion too. Could you let me know if it's okay for me to use these approches:

https://github.com/theochem/Selector/pull/212#discussion_r1645328396

https://github.com/theochem/Selector/pull/212#discussion_r1645332640

FanwangM commented 1 week ago

For now, I think we can focus on the web app itself and then we will deal with the docker image later. We can just comment on the related part. We may end up hosting the docker images with GitHub Packages.https://docs.github.com/en/actions/publishing-packages/publishing-docker-images#publishing-images-to-github-packages.

Can you update the codes based on https://huggingface.co/spaces/QCDevs/selector? Thanks for the hard work. @JackyZzZz

JackyZzZz commented 1 week ago

Hi @FanwangM, thank you for the comment!

That makes sense since this way the user can stay on our Github page to get the docker image without going to another site. I will also take a look at how to make this work too basides working on other interfaces. We can discuss this in our next meeting too since it looks like there are somewhat more setup for publishing images on Github.

I will update the workflow to push it to our official space. However, I noticed that the space you created is a streamlit space. Could you update this to a docker space maybe? I think streamlit space are for simpler apps so that the space will be looking for that app.py in the root of the folder to build the application. In our case it will be a package with our streamlit code in streamlit_app folder. So a docker space will use the Dockerfile we have in the root of our repo to start the interface application in a docker container where our package resides, which is also the space I've been using for testing.

Also, could you set the HF_TOKEN secret in the setting of our repo. This is required so that the workflow have appropriate permission to push to the HuggingFace. You can generate the token from HuggingFace and paste it in Github secrets.

Let me know if there is any questions~

FanwangM commented 1 week ago

Hi @FanwangM, thank you for the comment!

That makes sense since this way the user can stay on our Github page to get the docker image without going to another site. I will also take a look at how to make this work too basides working on other interfaces. We can discuss this in our next meeting too since it looks like there are somewhat more setup for publishing images on Github.

I will update the workflow to push it to our official space. However, I noticed that the space you created is a streamlit space. Could you update this to a docker space maybe? I think streamlit space are for simpler apps so that the space will be looking for that app.py in the root of the folder to build the application. In our case it will be a package with our streamlit code in streamlit_app folder. So a docker space will use the Dockerfile we have in the root of our repo to start the interface application in a docker container where our package resides, which is also the space I've been using for testing.

Also, could you set the HF_TOKEN secret in the setting of our repo. This is required so that the workflow have appropriate permission to push to the HuggingFace. You can generate the token from HuggingFace and paste it in Github secrets.

Let me know if there is any questions~

The space has been converted to a docker space and the HF_TOKEN secret is set up as well. Please lt me know if there is a problem. @JackyZzZz

JackyZzZz commented 1 week ago

Hi @FanwangM, I commented out the code in the workflow for building and pushing docker image. I also updated the command for pushing to HuggingFace. Could you run the workflow again to see if it works? I think it should work now.

JackyZzZz commented 1 week ago

Hi @FanwangM ,I figured that we should use HEAD to represent the current branch where the pull request is on for pushing to the main branch on HuggingFace. This should fix the problem now hopefully. However, I am starting to think should we have like a middle layer for deployment testing? In the future, if other contributors make pull requests about the interface, I believe that we wouldn't want the changes to go live directly before it has been fully reviewed and tested. I can take a look into making it work as well once I finish all the interfaces.

FanwangM commented 1 week ago

Hi @FanwangM ,I figured that we should use HEAD to represent the current branch where the pull request is on for pushing to the main branch on HuggingFace. This should fix the problem now hopefully. However, I am starting to think should we have like a middle layer for deployment testing? In the future, if other contributors make pull requests about the interface, I believe that we wouldn't want the changes to go live directly before it has been fully reviewed and tested. I can take a look into making it work as well once I finish all the interfaces.

This is a good and important point! Thanks for bringing it up. We should have this on the to-do list.

JackyZzZz commented 1 week ago

Hi @FanwangM, I noticed the pushing to HuggingFace is still failing for some reason. Just curious, did you setup the HF_TOKEN as the password of the account or the user access token generated by HuggingFace. From the error message, it looks like it's setting up using the password right now, which is not supported anymore.

image

You can refer to this tutorial for how to generate the access token up if you haven't already. We also need to give all the necessary permissions to the token such as write access etc to allow our github action to be able to push.

FanwangM commented 1 week ago

Hi @FanwangM, I noticed the pushing to HuggingFace is still failing for some reason. Just curious, did you setup the HF_TOKEN as the password of the account or the user access token generated by HuggingFace. From the error message, it looks like it's setting up using the password right now, which is not supported anymore.

image

You can refer to this tutorial for how to generate the access token up if you haven't already. We also need to give all the necessary permissions to the token such as write access etc to allow our github action to be able to push.

Thanks for checking this. But I did set up the HF_token as the repo secret.

Did you successfully deployed the application from your end using your own HF account before?

JackyZzZz commented 1 week ago

Yes. I just deployed another version on my own space minutes ago. Did you set it up as HF_TOKEN or HF_token? I think it's also case-sensitive, so these two are not the same thing.

FanwangM commented 1 week ago

We have HF_TOKEN.