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

refactor(publisher): update the target bucket key for publishing to fileserver #211

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

wuhuizuo
Copy link
Contributor

  • from centos7/*.tar.gz to <os>_<arch>/*.tar.gz

Signed-off-by: wuhuizuo [email protected]

…ileserver

- from `centos7/*.tar.gz` to `<os>_<arch>/*.tar.gz`

Signed-off-by: wuhuizuo <[email protected]>
@ti-chi-bot ti-chi-bot bot requested a review from purelind December 12, 2024 11:13
Copy link

ti-chi-bot bot commented Dec 12, 2024

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

This pull request is about refactoring some parts of the publisher package to update the target bucket key for publishing to the file server. The key change is from centos7/*.tar.gz to <os>_<arch>/*.tar.gz.

Key Changes:

  1. The regular expression reTagOsArchSuffix is modified to extract the OS and architecture from the tag.
  2. A new helper function getFsEntryPointPrefix(tag string) is added to get the entry point prefix from the tag.
  3. The analyzeFsFromOciArtifact, analyzeFsFromOciArtifactForTiDBTools, analyzeFsFromOciArtifactForTiDBBinlog, and transformFsEntryPoint functions are updated to use the new entry point prefix.
  4. The test files are updated to reflect these changes.

Potential Problems:

  1. The new regular expression reTagOsArchSuffix only supports Linux and Darwin (MacOS), and AMD64 and ARM64 architectures. If there are other OS or architecture types, they will not be matched.
  2. The getFsEntryPointPrefix function assumes that the tag will always have a matching OS and architecture suffix. If the tag does not have a matching suffix, it defaults to "centos7". This may not be the desired behavior if the tag does not follow the expected format.
  3. The transformFsEntryPoint function is updated to take an additional parameter prefix. This could potentially break other parts of the code that use this function.

Suggestions for Fixes:

  1. Consider expanding the regular expression to support more OS and architecture types if needed.
  2. Implement error handling or a fallback mechanism in getFsEntryPointPrefix for cases where the tag does not have a matching suffix.
  3. Check all uses of the transformFsEntryPoint function and update them to pass the new prefix parameter.

@ti-chi-bot ti-chi-bot bot added the size/L label Dec 12, 2024
@wuhuizuo
Copy link
Contributor Author

/test ?

Copy link

ti-chi-bot bot commented Dec 12, 2024

@wuhuizuo: The following commands are available to trigger required jobs:

/test pull-security-scan

In response to this:

/test ?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@wuhuizuo
Copy link
Contributor Author

/test pull-security-scan

@wuhuizuo
Copy link
Contributor Author

/retest

1 similar comment
@wuhuizuo
Copy link
Contributor Author

/retest

@wuhuizuo
Copy link
Contributor Author

/approve

Copy link

ti-chi-bot bot commented Dec 12, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wuhuizuo

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

@ti-chi-bot ti-chi-bot bot added the approved label Dec 12, 2024
@ti-chi-bot ti-chi-bot bot merged commit d78ec05 into main Dec 12, 2024
8 checks passed
@ti-chi-bot ti-chi-bot bot deleted the feature/update-file-path branch December 12, 2024 11:34
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.

1 participant