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(deps): update golang image for mysql_connector_test #3021

Merged
merged 2 commits into from
Jul 4, 2024

Conversation

djshow832
Copy link
Contributor

@djshow832 djshow832 commented Jul 3, 2024

User description

After upgrading tidb deps to v1.1.0 in tiproxy, the test mysql_connector_test reports:

/go/pkg/mod/github.com/pingcap/tidb@v1.1.0-beta.0.20240428083427-66ba419636ce/pkg/types/compare.go:18:2: package cmp is not in GOROOT (/usr/local/go/src/cmp)
note: imported by a module that requires go 1.21
/go/pkg/mod/github.com/pingcap/tidb@v1.1.0-beta.0.20240428083427-66ba419636ce/pkg/util/stmtsummary/v2/stmtsummary.go:20:2: package maps is not in GOROOT (/usr/local/go/src/maps)
note: imported by a module that requires go 1.21
/go/pkg/mod/github.com/tikv/client-go/v2@v2.0.8-0.20240424052342-0229f4077f0c/internal/locate/region_cache.go:44:2: package slices is not in GOROOT (/usr/local/go/src/slices)
note: imported by a module that requires go 1.21

Maybe I should upgrade the golang image.


PR Type

dependencies, configuration changes


Description

  • Updated the Golang image version in tidb-test pipeline configuration to golang-with-gcc11:1.21.
  • Updated the Golang image version in tiproxy pipeline configuration to golang-with-gcc11:1.21.
  • These changes address issues with missing packages in the GOROOT for the mysql_connector_test.

Changes walkthrough 📝

Relevant files
Dependencies
pod-pull_tiproxy_mysql_connector_test.yaml
Update Golang image version in tidb-test pipeline configuration

pipelines/PingCAP-QE/tidb-test/latest/pod-pull_tiproxy_mysql_connector_test.yaml

  • Updated the Golang image version from centos7_golang-1.21:latest to
    golang-with-gcc11:1.21.
  • +1/-1     
    pod-pull_mysql_connector_test.yaml
    Update Golang image version in tiproxy pipeline configuration

    pipelines/pingcap/tiproxy/latest/pod-pull_mysql_connector_test.yaml

  • Updated the Golang image version from golang-with-gcc11:1.20 to
    golang-with-gcc11:1.21.
  • +1/-1     

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

    Copy link

    ti-chi-bot bot commented Jul 3, 2024

    I have already done a preliminary review for you, and I hope to help you do a better job.

    Key Changes:

    • The golang image version has been updated from 1.20 to 1.21 in the pod-pull_mysql_connector_test.yaml file.
    • The updated image is now hub.pingcap.net/jenkins/centos7_golang-1.21:latest.

    Potential Problems:

    • It's unclear if the new image has been tested and verified to work with the mysql_connector_test.
    • There may be other dependencies that need to be updated in order to work with the new golang version.

    Fixing Suggestions:

    • Before merging the PR, it's important to ensure that the mysql_connector_test works with the new golang image version.
    • It may be necessary to update other dependencies to ensure compatibility with the new golang version.
    • It would be helpful to add more information to the PR description, such as how the updated image was chosen and if any other changes were made to accommodate the new version.

    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 3, 2024

    PR Reviewer Guide 🔍

    (Review updated until commit 224d27e)

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

    None

    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 3, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Pin the version of the Golang image to ensure consistent and stable builds

    Consider pinning the version of the Golang image instead of using the 'latest' tag. Using
    a specific version ensures that the builds are repeatable and stable over time, avoiding
    unexpected behaviors due to changes in the 'latest' image.

    pipelines/pingcap/tiproxy/latest/pod-pull_mysql_connector_test.yaml [8]

    -image: "hub.pingcap.net/jenkins/centos7_golang-1.21:latest"
    +image: "hub.pingcap.net/jenkins/centos7_golang-1.21:1.21.0"
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Pinning the version of the Golang image is a best practice that ensures builds are repeatable and stable over time. This avoids unexpected behaviors due to changes in the 'latest' image.

    9

    @djshow832 djshow832 marked this pull request as draft July 3, 2024 08:00
    @djshow832 djshow832 marked this pull request as ready for review July 4, 2024 04:03
    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 4, 2024

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    Persistent review updated to latest commit 224d27e

    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 4, 2024

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Use a more specific tag for Docker images to ensure reproducibility

    Consider using a more specific tag instead of "1.21" for the Docker image to ensure that
    the builds are reproducible and consistent. Using a specific tag or a digest ensures that
    the exact same image is used every time, which is crucial for debugging and reliability in
    production environments.

    pipelines/PingCAP-QE/tidb-test/latest/pod-pull_tiproxy_mysql_connector_test.yaml [8]

    -image: "hub.pingcap.net/jenkins/golang-with-gcc11:1.21"
    +image: "hub.pingcap.net/jenkins/golang-with-gcc11:1.21.3"
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Using a more specific tag for Docker images is a best practice that ensures builds are reproducible and consistent, which is crucial for debugging and reliability in production environments.

    9
    Enhancement
    Update the Docker image tag to the latest minor version for consistency and feature completeness

    To avoid potential incompatibilities and ensure that all features and fixes are included,
    consider updating the image tag to match the latest minor version used in other similar
    configurations, if applicable.

    pipelines/pingcap/tiproxy/latest/pod-pull_mysql_connector_test.yaml [8]

    -image: "hub.pingcap.net/jenkins/golang-with-gcc11:1.21"
    +image: "hub.pingcap.net/jenkins/golang-with-gcc11:1.21.3"
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Updating the Docker image tag to the latest minor version helps avoid potential incompatibilities and ensures that all features and fixes are included, promoting consistency across similar configurations.

    8

    Copy link
    Collaborator

    @purelind purelind left a comment

    Choose a reason for hiding this comment

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

    /lgtm

    @ti-chi-bot ti-chi-bot bot added the lgtm label Jul 4, 2024
    Copy link

    ti-chi-bot bot commented Jul 4, 2024

    [APPROVALNOTIFIER] This PR is APPROVED

    This pull-request has been approved by: purelind

    The full list of commands accepted by this bot can be found here.

    The pull request process is described here

    Needs approval from an approver in each of these files:

    Approvers can indicate their approval by writing /approve in a comment
    Approvers can cancel approval by writing /approve cancel in a comment

    Copy link

    ti-chi-bot bot commented Jul 4, 2024

    [LGTM Timeline notifier]

    Timeline:

    • 2024-07-04 04:09:05.396788309 +0000 UTC m=+1470271.882277136: ☑️ agreed by purelind.

    @ti-chi-bot ti-chi-bot bot added the approved label Jul 4, 2024
    @ti-chi-bot ti-chi-bot bot merged commit 8ec524e into PingCAP-QE:main Jul 4, 2024
    1 check passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    Status: Done
    Development

    Successfully merging this pull request may close these issues.

    2 participants