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: github action will miss ref and use default branch #3822

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wuwentao
Copy link

Issue:

we want to migrate a domain name, PR: wuwentao/midea_ac_lan#159
PR submit from fork repo with different branch name.

github action uses hacs/action@main to verify PR
it only return “Error: Not Found”
no any ERROR log or step output in debug mode.

after fork it and add too many log info to print the step and args, found it caused by ref args miss:

  1. action.py run hacs.async_register_repository() with ref args
  2. async_register_repository defined in custom_components/hacs/base.py line 562, it call await repository.async_registration(ref), also include ref args
  3. and repository.async_registration(ref) defined in custom_components/hacs/repositories/base.py, in line 842, it call self.validate_repository() without ref args, but self.ref is available
  4. validate_repository() defined in custom_components/hacs/repositories/integration.py, in line 73, it call common_validate in custom_components/hacs/repositories/base.py, and it will run this line if manifest := await self.async_get_hacs_json():, it missed ref args, so add arg: ref=self.ref to fix it.
  5. but it will not return error during domain rename, as it using default branch hacs.json. it check pass, continue the remaining jobs, in custom_components/hacs/repositories/integration.py after line 73 in validate_repository().
  6. error found in line 94: if manifest := await self.async_get_integration_manifest(): , also missed ref args and use default branch value.
  7. in async_get_integration_manifest, as there is domain rename and dir rename, default branch don't have this dir and mainfest.json, it continue using default branch and call response = await self.hacs.async_github_api_method() in line 183
  8. async_github_api_method() definded in custom_components/hacs/base.py line 492, and try job can't pass with a exception: Not Found, so only print this error log.

checked with the same async_get_integration_manifest and async_get_hacs_json, add ref=self.ref to run it in normal branch and action.

issue fixed.

in addtion, also remove some minior error , like duplicate ## comments, remove from .utils.json import json_loads as import but not used.

please help to review and confirm whether it's the expect result.

github action error log:
https://github.com/wuwentao/midea_ac_lan/actions/runs/9672246348/job/26686312538

after add the ref args fix, it can report the expect ERROR log as below:
https://github.com/wuwentao/midea_ac_lan/actions/runs/9672246348/job/26687155544

Thanks

@ludeeus
Copy link
Member

ludeeus commented Jun 26, 2024

Protip: do NOT change the domain.
This will cause you a lot of issues.
This is not even being done inside of core because of it.

@wuwentao
Copy link
Author

Protip: do NOT change the domain. This will cause you a lot of issues. This is not even being done inside of core because of it.

Thanks, it's really not a good choice, as the old domain owner not maintain it now, we want to work with a new domain, this domain migrate just want to save exist old user configs/entiries/devices.

if we drop it, old integration can't works in new HA, also no any bug fix, and they should start from zero with all the old configs and device history.

so we want to have a try, test it and confirm it what we can do, we have finished some config_entries/device_entries update in new domain, but still under test.

found this hacs/action failed and try to fix it.

@ludeeus
Copy link
Member

ludeeus commented Jun 26, 2024

@coderabbitai review

Copy link
Contributor

coderabbitai bot commented Jun 26, 2024

Walkthrough

Walkthrough

The changes improve code readability by standardizing comment formatting and enhancing method functionalities in the HACS (Home Assistant Custom Component Store) project. Specifically, comments were streamlined, unused imports were removed, and methods were updated for more accurate data retrieval. These modifications collectively contribute to a cleaner, more efficient, and robust codebase.

Changes

File(s) Change Summary
custom_components/hacs/__init__.py, custom_components/hacs/repositories/integration.py Updated comments from double ## to single # for improved readability.
custom_components/hacs/base.py Removed unused import of json_loads.
custom_components/hacs/repositories/base.py, custom_components/hacs/repositories/integration.py Updated methods to use version-specific calls for retrieving HACS JSON and integration manifests, enhancing data accuracy.

Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

Commits

Files that changed from the base of the PR and between 568f4da and cfbc9d3.

Files selected for processing (4)
  • custom_components/hacs/init.py (2 hunks)
  • custom_components/hacs/base.py (1 hunks)
  • custom_components/hacs/repositories/base.py (1 hunks)
  • custom_components/hacs/repositories/integration.py (1 hunks)
Additional comments not posted (3)
custom_components/hacs/__init__.py (1)

Line range hint 89-99: Comment style change approved.

The change from double hash marks (##) to a single hash mark (#) for comments is a good practice for consistency and readability.

custom_components/hacs/repositories/integration.py (1)

95-95: Version-specific manifest retrieval approved.

The change to use get_integration_manifest(version=self.ref) enhances flexibility by allowing version-specific retrieval. Ensure that all calls to this method are updated accordingly.

custom_components/hacs/repositories/base.py (1)

479-479: Version-specific HACS JSON retrieval approved.

The use of get_hacs_json(version=self.ref) in common_validate allows for more precise version control. Ensure that all related logic is updated to handle version-specific data correctly.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

action/action.py Outdated Show resolved Hide resolved
Copy link
Member

@ludeeus ludeeus left a comment

Choose a reason for hiding this comment

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

changes requested, and rebase needed.

@ludeeus ludeeus marked this pull request as draft July 19, 2024 16:15
@wuwentao wuwentao force-pushed the main branch 2 times, most recently from e43ec38 to 172dfae Compare July 22, 2024 09:22
@wuwentao
Copy link
Author

Hi @ludeeus , sorry for the delay, finished update based on review comments. please help to check again

@wuwentao wuwentao marked this pull request as ready for review August 21, 2024 02:21
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