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

fix: update yml and .env files #10

Merged
merged 4 commits into from
Sep 25, 2024
Merged

Conversation

saalim-mushtaq
Copy link
Collaborator

@saalim-mushtaq saalim-mushtaq commented Aug 8, 2024

PR Type

Enhancement, Configuration changes


Description

  • Added new domain environment variables for admin, customer, and reseller portals in .env.
  • Added a new GitHub Actions workflow for PR-Agent in .github/workflows/pr_agent_codium.yml.
  • Updated PostgreSQL image version and added services for admin, customer, and reseller portals in docker-compose.yml.
  • Added Google Analytics key environment variable in web-api.env.

Changes walkthrough 📝

Relevant files
Configuration changes
.env
Add new domain environment variables                                         

.env

  • Added new domain environment variables for admin, customer, and
    reseller portals.
  • +3/-0     
    web-api.env
    Add Google Analytics key environment variable                       

    web-api.env

    • Added Google Analytics key environment variable.
    +2/-0     
    Enhancement
    pr_agent_codium.yml
    Add PR-Agent GitHub Actions workflow                                         

    .github/workflows/pr_agent_codium.yml

    • Added a new GitHub Actions workflow for PR-Agent.
    +9/-0     
    docker-compose.yml
    Update PostgreSQL version and add new services                     

    docker-compose.yml

  • Updated PostgreSQL image version.
  • Added services for admin, customer, and reseller portals.
  • +75/-2   

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @github-actions github-actions bot added enhancement New feature or request configuration changes labels Aug 8, 2024
    Copy link

    github-actions bot commented Aug 8, 2024

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No key issues to review

    Copy link

    github-actions bot commented Aug 8, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Implement health checks for service containers to ensure they are running correctly

    Add a health check to the admin-portal, customer-portal, and reseller-portal
    services to ensure that the containers are healthy and running as expected. This can
    help in managing container orchestration better.

    docker-compose.yml [153-202]

     admin-portal:
       image: cryptlex/admin-portal:latest
    +  healthcheck:
    +    test: ["CMD", "curl", "-f", "http://localhost:4400"]
    +    interval: 1m30s
    +    timeout: 10s
    +    retries: 3
    +    start_period: 40s
     customer-portal:
       image: cryptlex/customer-portal:latest
    +  healthcheck:
    +    test: ["CMD", "curl", "-f", "http://localhost:4401"]
    +    interval: 1m30s
    +    timeout: 10s
    +    retries: 3
    +    start_period: 40s
     reseller-portal:
       image: cryptlex/reseller-portal:latest
    +  healthcheck:
    +    test: ["CMD", "curl", "-f", "http://localhost:4402"]
    +    interval: 1m30s
    +    timeout: 10s
    +    retries: 3
    +    start_period: 40s
     
    Suggestion importance[1-10]: 10

    Why: Adding health checks to the services is crucial for ensuring that the containers are healthy and running as expected, which improves the reliability and manageability of the container orchestration.

    10
    Best practice
    Use specific version tags for Docker images to ensure deployment consistency

    Consider using a specific version tag for the cryptlex/admin-portal,
    cryptlex/customer-portal, and cryptlex/reseller-portal images instead of latest to
    ensure consistent, predictable deployments and avoid potential issues with
    unexpected changes when the latest tag is updated.

    docker-compose.yml [154-202]

    -image: cryptlex/admin-portal:latest
    -image: cryptlex/customer-portal:latest
    -image: cryptlex/reseller-portal:latest
    +image: cryptlex/admin-portal:v1.0.0
    +image: cryptlex/customer-portal:v1.0.0
    +image: cryptlex/reseller-portal:v1.0.0
     
    Suggestion importance[1-10]: 9

    Why: Using specific version tags for Docker images is a best practice that ensures consistent and predictable deployments, avoiding potential issues with unexpected changes when the latest tag is updated.

    9
    Performance
    Restrict workflow triggers to specific branches to optimize CI/CD resource usage

    Specify the branches for which the workflow should run under the on section to
    prevent unnecessary runs on all branches and tags. This can help in saving CI/CD
    resources and faster builds.

    .github/workflows/pr_agent_codium.yml [3-5]

     on:
       pull_request:
    +    branches:
    +      - main
       push:
    +    branches:
    +      - main
     
    Suggestion importance[1-10]: 8

    Why: Specifying branches for which the workflow should run helps in saving CI/CD resources and results in faster builds by preventing unnecessary runs on all branches and tags.

    8
    Maintainability
    Externalize logging configuration to enhance flexibility and maintainability

    It's recommended to externalize the logging configuration to a separate file or
    environment variables to make it easier to change logging levels or methods without
    modifying the docker-compose file directly.

    docker-compose.yml [168-218]

     logging:
    -  driver: "json-file"
    +  driver: ${LOG_DRIVER}
       options:
    -    max-size: "20m"
    +    max-size: ${LOG_MAX_SIZE}
     
    Suggestion importance[1-10]: 7

    Why: Externalizing logging configuration to environment variables or a separate file enhances flexibility and maintainability, making it easier to change logging settings without modifying the docker-compose file directly.

    7

    image: cryptlex/admin-portal:latest
    depends_on:
    - web-api
    env_file: web-portals.env
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    does this file exist now?

    @saalim-mushtaq
    Copy link
    Collaborator Author

    @adnan-kamili

    Copy link

    .env Outdated
    @@ -10,6 +10,9 @@ [email protected]
    WEB_API_DOMAIN=cryptlex-api.mycompany.com
    DASHBOARD_DOMAIN=cryptlex-app.mycompany.com
    RELEASE_SERVER_DOMAIN=cryptlex-releases.mycompany.com
    ADMIN_PORTAL_DOMAIN=cryptlex-admin.mycompany.com
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    admin-portal.mycompany.com

    and similarly for others

    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

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

    @adnan-kamili do we have to remove the dashboard.env file?

    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

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

    what about "WEB_API_DOMAIN" "DASHBOARD_DOMAIN" and "RELEASE_SERVER_DOMAIN"

    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    these need not be changed

    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    @adnan-kamili do we have to remove the dashboard.env file?

    After customers migrate to the new portal, this file can be removed. In on-premise deployment docs we don't have to mention this file

    @adnan-kamili adnan-kamili merged commit 8d226a4 into master Sep 25, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants