Feat/talk2scholars docker update #118
Conversation
There was a problem hiding this comment.
Thx @ansh-info for setting this up. Looks very good 👍
I don't have any comments but I have a request in the workflow file. Let me know if it (or a subset) would be possible in this PR? Thx
There was a problem hiding this comment.
Would it be possible to:
- Have this workflow automatically run after the RELEASE workflow was successful.
- Separate it into different jobs based on the agent, and invoke the jobs corresponding to the changes made. For example, if the changes were made to T2S, then only the docker job of T2S is invoked.
- Automatically increment the version (and not use v1 always) :-)
There was a problem hiding this comment.
Hi @gurdeep330, Thank you for the review -> Yes we can add these changes, but I want your review first
Please review the below, also regarding the last point we can use GitHub tags, please let me know if this solution will work about tags(will be fetched from the GitHub environment variable)
1. Trigger This Workflow After the RELEASE Workflow Completes
Solution: Modify the workflow trigger to run after release.yml completes successfully using workflow_run:
Update ci.yml:
on:
workflow_run:
workflows: ["RELEASE"]
types:
- completed
workflow_dispatch:2. Separate Jobs by Agent and Run Only for Relevant Changes
Solution:
Modify the workflow to conditionally run jobs based on changes in talk2biomodels or talk2scholars.
Update ci.yml:
jobs:
detect-changes:
runs-on: ubuntu-latest
outputs:
build_t2b: ${{ steps.filter.outputs.talk2biomodels }}
build_t2s: ${{ steps.filter.outputs.talk2scholars }}
steps:
- name: Checkout Repository
uses: actions/checkout@v4
- name: Detect Changes
id: filter
uses: dorny/paths-filter@v2
with:
filters: |
talk2biomodels:
- 'aiagents4pharma/talk2biomodels/**'
talk2scholars:
- 'aiagents4pharma/talk2scholars/**'
docker-build-t2b:
needs: detect-changes
if: needs.detect-changes.outputs.build_t2b == 'true'
runs-on: ubuntu-latest
steps:
- name: Checkout Repository
uses: actions/checkout@v4
- name: Login to Docker Hub
uses: docker/login-action@v3
with:
username: ${{ vars.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_TOKEN }}
- name: Build and Push Talk2Biomodels
uses: docker/build-push-action@v6
with:
file: aiagents4pharma/talk2biomodels/Dockerfile
push: true
tags: virtualpatientengine/talk2biomodels:latest
docker-build-t2s:
needs: detect-changes
if: needs.detect-changes.outputs.build_t2s == 'true'
runs-on: ubuntu-latest
steps:
- name: Checkout Repository
uses: actions/checkout@v4
- name: Login to Docker Hub
uses: docker/login-action@v3
with:
username: ${{ vars.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_TOKEN }}
- name: Build and Push Talk2Scholars
uses: docker/build-push-action@v6
with:
file: aiagents4pharma/talk2scholars/Dockerfile
push: true
tags: virtualpatientengine/talk2scholars:latest3. Automatically Increment the Version Instead of Always Using v1
Solution:
Modify the workflow to auto-increment the version using the latest GitHub tag.
Update the Build and Push steps in both jobs:
- name: Get Version
id: version
run: echo "VERSION=$(git describe --tags --abbrev=0)" >> $GITHUB_ENV
- name: Build and Push
uses: docker/build-push-action@v6
with:
file: aiagents4pharma/talk2biomodels/Dockerfile
push: true
tags: |
virtualpatientengine/talk2biomodels:${{ env.VERSION }}
virtualpatientengine/talk2biomodels:latest- Uses the latest Git tag as the version number instead of hardcoding
v1.
There was a problem hiding this comment.
Yes, this looks fine to me. But could you double-check these tests for different conditions by running them in your forked repo? Thx
…trigger without any additional conditions.
|
HI @gurdeep330 please check the screenshot, The first point you mentioned is completed Let me explain what's happening:
The error "denied: requested access to the resource is denied" means that my GitHub Actions workflow doesn't have permission to push to that Docker Hub repository. This is expected in a fork scenario |
|
@gurdeep330 Please check the screenshot, I am able to achive the second point you mentioned as well: 
Previously, with the matrix strategy, when the first job failed (talk2biomodels), the second job (talk2scholars) was automatically canceled. This was because the matrix jobs were part of a single logical job.
This confirms that:
The only error happening now is still the Docker Hub permission issue, which is expected in my fork scenario and doesn't indicate any problem with the workflow structure itself. |
… specific directories
|
@gurdeep330 Your 3rd point is also completed, Please check the attached screenshots We have a version check workflow added now
Checks for the latest tag from RELEASE workflow
Build summary -> both got build
Implementation of step 3: automatically incrementing the version instead of using a static version. Since our project is already using semantic-release in the RELEASE workflow, we leveraged that work and ensure the Docker images use the correctly incremented versions. I've implemented step 3, adding automatic version incrementing with a sophisticated approach: Key Improvements:
This approach gives you several benefits:
This implementation completes the third and final step requested in the PR review. The workflow now:
Let me know if you'd like any further refinements to this implementation! |
|
Hello @gurdeep330, Just to make sure, I change the username to my GitHub credentials on my test fork/test brach, and everything ran successfully. Please check the screenshots bellow(-> This was just to test I have removed the images from my docker hub😊): Github Actions Overview
Docker Hub preview
|
|
Thanks very much @ansh-info This is very nicely done 💯 I'll run a few tests after the merge is complete. Great work 🥇 You just need an approval from @dmccloskey :-) |
|
🎉 This PR is included in version 1.24.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
For authors
Both Build and tested locally
Description
This pull request introduces Docker support for the
talk2scholarsmodule, aligning it with the existingtalk2biomodelssetup. The changes include:talk2scholarspython:3.12-slimas the base imageg++andbuild-essentialfor compilingpcst_fast8501for StreamlitAdditional features on request of @gurdeep330 -> Please check the conversation thread below
ci.yml) for improved CI/CDAdded trigger to automatically run after the RELEASE workflow completes
Implemented module-specific build jobs with path filtering
talk2biomodelsandtalk2scholarsdorny/paths-filterto detect changes in specific directoriesAdded automatic versioning for Docker images
latesttags to all imagesREADME.mdtalk2biomodelsandtalk2scholarsContext and Motivation
The goal of this update is to streamline the deployment process for both
talk2biomodelsandtalk2scholars, ensuring a consistent and scalable setup. By integratingtalk2scholarsinto the CI/CD pipeline, the module can now be built and deployed automatically alongsidetalk2biomodels.The enhanced CI workflow improves efficiency by only building Docker images for modules that have changed and automatically handling versioning, reducing maintenance overhead and ensuring consistency across releases.
Dependencies
OPENAI_API_KEY,ZOTERO_API_KEY,ZOTERO_USER_ID,NVIDIA_API_KEY) must be set when running the container.Fixes # (issue) Mention the issue number.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests you conducted to verify your changes. These may involve creating new test scripts or updating existing ones.
testsfoldertests/testX.py)Checklist
testsfolder) that prove my fix is effective or that my feature worksFor reviewers
Checklist pre-approval
Checklist post-approval
developintomain? If so, please make sure to add a prefix (feat/fix/chore) and/or a suffix BREAKING CHANGE (if it's a major release) to your commit message.Checklist post-merge
developintomainand is it suppose to run an automated release workflow (if applicable)? If so, please make sure to check under the "Actions" tab to see if the workflow has been initiated, and return later to verify that it has completed successfully.