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

Revert "build(deps): Update dependencies charts" #234

Merged
merged 3 commits into from
Oct 3, 2024

Conversation

knowlet
Copy link
Contributor

@knowlet knowlet commented Oct 3, 2024

User description

Reverts #232


PR Type

Bug fix


Description

  • This PR reverts changes made in the previous update to the Helm charts.
  • Restores the minio dependency version to a stable version.
  • Updates conditions in the report deployment and service templates to use .Values.ui.enabled.
  • Modifies the postgresql image repository and tag for consistency.

Changes walkthrough 📝

Relevant files
Bug fix
Chart.yaml
Revert minio dependency version to previous stable version

charts/agh3/Chart.yaml

  • Reverted minio dependency version from 14.7.15 to 12.2.4.
+1/-1     
report-deployment.yml
Update report deployment condition                                             

charts/agh3/templates/report/report-deployment.yml

  • Changed condition from .Values.report.enabled to .Values.ui.enabled.
  • +1/-1     
    report-service.yml
    Update report service condition                                                   

    charts/agh3/templates/report/report-service.yml

  • Changed condition from .Values.report.enabled to .Values.ui.enabled.
  • +1/-1     
    values.yaml
    Update postgresql image repository and tag                             

    charts/agh3/values.yaml

  • Updated postgresql image repository from docker/bitnami/postgresql to
    leukocyte-lab/postgresql.
  • Updated postgresql image tag to
    v0.1.2-postgres-15.1.0-pgroonga-2.4.2-debian.
  • +2/-2     

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

    @ai-themis
    Copy link

    ai-themis bot commented Oct 3, 2024

    PR Reviewer Guide 🔍

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

    Code Smell
    The condition .Values.ui.enabled is used in the report deployment template, which might not be the intended condition. Verify if .Values.report.enabled should be used instead.

    Code Smell
    The condition .Values.ui.enabled is used in the report service template, which might not be the intended condition. Verify if .Values.report.enabled should be used instead.

    @ai-themis
    Copy link

    ai-themis bot commented Oct 3, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Verify the compatibility of the downgraded minio version with the system

    Ensure that the version of minio specified is correct and compatible with the rest
    of the system, as downgrading from 14.7.15 to 12.2.4 might introduce compatibility
    issues or missing features.

    charts/agh3/Chart.yaml [28]

    -version: 12.2.4
    +version: 14.7.15
     
    Suggestion importance[1-10]: 8

    Why: The suggestion addresses a potential compatibility issue due to the significant version downgrade of minio, which could impact functionality.

    8
    Validate the PostgreSQL image repository and tag for compatibility

    Double-check the repository and tag for the PostgreSQL image to ensure they are
    correct and compatible with the rest of the system, as changing from
    docker/bitnami/postgresql to leukocyte-lab/postgresql might introduce unexpected
    behavior.

    charts/agh3/values.yaml [216-217]

    -repository: leukocyte-lab/postgresql
    -tag: v0.1.2-postgres-15.1.0-pgroonga-2.4.2-debian
    +repository: docker/bitnami/postgresql
    +tag: 15
     
    Suggestion importance[1-10]: 8

    Why: The suggestion highlights a critical change in the PostgreSQL image repository and tag, which could lead to unexpected behavior if not properly validated.

    8
    Possible bug
    Re-evaluate the conditional logic for enabling the deployment

    Consider using a more descriptive conditional check for enabling the deployment, as
    changing from report.enabled to ui.enabled might affect the intended functionality.

    charts/agh3/templates/report/report-deployment.yml [1]

    -{{- if and .Values.ui.enabled }}
    +{{- if and .Values.report.enabled }}
     
    Suggestion importance[1-10]: 7

    Why: The change in the conditional check from report.enabled to ui.enabled could affect the deployment's intended behavior, warranting a review.

    7
    Verify the conditional logic for enabling the service

    Review the conditional logic for enabling the service, as changing from
    report.enabled to ui.enabled might not align with the intended service
    configuration.

    charts/agh3/templates/report/report-service.yml [1]

    -{{- if and .Values.ui.enabled }}
    +{{- if and .Values.report.enabled }}
     
    Suggestion importance[1-10]: 7

    Why: Similar to the deployment, changing the conditional check from report.enabled to ui.enabled may misalign with the intended service configuration, thus needing verification.

    7

    @knowlet knowlet requested a review from Aries0d0f October 3, 2024 09:06
    @Aries0d0f Aries0d0f merged commit 6d1930c into main Oct 3, 2024
    1 check passed
    @Aries0d0f Aries0d0f deleted the revert-232-build/upgrade-minio branch October 3, 2024 09:09
    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