-
Couldn't load subscription status.
- Fork 24
Private repo support #40
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
Conversation
also fix some spelling errors
From my test, it seems there is a new error
INFO:CPP Linter:Running "clang-format-10 -style=file --output-replacements-xml demo.cpp"
Posting comment(s)
Traceback (most recent call last):
File "/usr/lib/python3.8/runpy.py", line 194, in _run_module_as_main
return _run_code(code, main_globals, None,
File "/usr/lib/python3.8/runpy.py", line 87, in _run_code
exec(code, run_globals)
File "/usr/local/lib/python3.8/dist-packages/cpp_linter/run.py", line 764, in <module>
main()
File "/usr/local/lib/python3.8/dist-packages/cpp_linter/run.py", line 7[58](https://github.com/shenxianpeng/test-cpp-linter-action/runs/5637983841?check_suite_focus=true#step:4:58), in main
post_results(False) # False is hard-coded to disable diff comments.
File "/usr/local/lib/python3.8/dist-packages/cpp_linter/run.py", line 639, in post_results
checks_passed = post_pr_comment(base_url, user_id)
File "/usr/local/lib/python3.8/dist-packages/cpp_linter/run.py", line [60](https://github.com/shenxianpeng/test-cpp-linter-action/runs/5637983841?check_suite_focus=true#step:4:60)8, in post_pr_comment
comments_url = base_url + f'issues/{Globals.EVENT_PAYLOAD["number"]}/comments'
KeyError: 'number'
More output: https://github.com/shenxianpeng/test-cpp-linter-action/runs/5637983841?check_suite_focus=true
I have added you to the test repo https://github.com/shenxianpeng/test-cpp-linter-action
The error is strange because I didn't change anything with posting a PR comment. Maybe they changed the information available in the event payload provided to the runner.
I'll use the test repo to explore this more...
I think you stumbled onto a different issue. Seems that the PR number was only available (via the event_payload.json present on the workflow runner) if the files-changed-only option was set to true. I changed this to be available despite the options (or event type). Now I'm getting a different error:
Traceback (most recent call last):
File "/usr/lib/python3.8/runpy.py", line 194, in _run_module_as_main
return _run_code(code, main_globals, None,
File "/usr/lib/python3.8/runpy.py", line 87, in _run_code
exec(code, run_globals)
File "/usr/local/lib/python3.8/dist-packages/cpp_linter/run.py", line 765, in <module>
main()
File "/usr/local/lib/python3.8/dist-packages/cpp_linter/run.py", line 759, in main
post_results(False) # False is hard-coded to disable diff comments.
File "/usr/local/lib/python3.8/dist-packages/cpp_linter/run.py", line 639, in post_results
checks_passed = post_pr_comment(base_url, user_id)
File "/usr/local/lib/python3.8/dist-packages/cpp_linter/run.py", line 609, in post_pr_comment
remove_bot_comments(comments_url, user_id)
File "/usr/local/lib/python3.8/dist-packages/cpp_linter/thread_comments.py", line 24, in remove_bot_comments
int(comment["user"]["id"]) == user_id
TypeError: string indices must be integers
I'm looking into this one now...
I'm not sure why, but it was getting a 404 when fetching comments for a thread that doesn't have any comments. Maybe they changed the REST API response behavior for this scenario??
ERROR:CPP Linter:response returned 404 message: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/reference/repos#list-commit-comments"}
When I ignore the 404 response, it is fine. After I re-run the workflow with existing comments (posted to the commit not the PR), it still gets a 404 response and posts another comment to the commit's thread (undesirable behavior). It is possible the URL is malformed somehow. Still investigating...
I didn't realize you made the test repo private! That context helps. I think the token is required to fetch comments for a private repo. But I'm still looking into a new error about examining which comment was created by the bot. The following results from using the token to fetch the comments for a thread, but it says the comment doesn't have a body (which seems wrong according to the REST API docs).
INFO:CPP Linter:comments_url: https://api.github.com/repos/shenxianpeng/test-cpp-linter-action/issues/10/comments
Traceback (most recent call last):
File "/usr/lib/python3.8/runpy.py", line 194, in _run_module_as_main
return _run_code(code, main_globals, None,
File "/usr/lib/python3.8/runpy.py", line 87, in _run_code
exec(code, run_globals)
File "/usr/local/lib/python3.8/dist-packages/cpp_linter/run.py", line 765, in <module>
main()
File "/usr/local/lib/python3.8/dist-packages/cpp_linter/run.py", line 759, in main
post_results(False) # False is hard-coded to disable diff comments.
File "/usr/local/lib/python3.8/dist-packages/cpp_linter/run.py", line 639, in post_results
checks_passed = post_pr_comment(base_url, user_id)
File "/usr/local/lib/python3.8/dist-packages/cpp_linter/run.py", line 609, in post_pr_comment
remove_bot_comments(comments_url, user_id)
File "/usr/local/lib/python3.8/dist-packages/cpp_linter/thread_comments.py", line 28, in remove_bot_comments
and comment["body"].startswith("<!-- cpp linter action -->")
KeyError: 'body'
We have a problem. I finally got a peek at what the comments request's response looks like. Instead of a body field, each comment on a thread has a body_text field, and it does not include the HTML comment we have been using as an identifier (<!-- cpp-linter-action -->). It also seems to have the emoji symbols encoded. So, instead of starting with the HTML comment, it starts with the magic wand emoji encoded as \ud83d\udcdc.
@JosephDrazen I'm not sure if we can support private repos the same way we've been supporting public repos.
The changes here will work for private repos but only when not using thread comments.
I can identify a private repo given the event's JSON payload. Then I can force thread-comments option to false (despite the user's configuration)...
JosephDrazen
commented
Mar 22, 2022
Thanks for looking into this and getting back to me quickly. Our team found a solution to the problem as well and can share it to you if you wish.
Yes please. If we can, we'll try to include it here too.
* add database option * Fix existing typos Co-authored-by: 2bndy5 <2bndy5@gmail.com> Co-authored-by: shenxianpeng <xianpeng.shen@gmail.com>
also fix some spelling errors
Is this PR waiting for a review? (I'm not in a hurry, just don't want to delay something because of me :) )
Sorry I forgot to finish the testing. My initial attempt to force-disable thread comments on private repos failed...
@shenxianpeng I've successfully forced thread comments to be disabled for private repos. Tested everything on the private test repo, and it works.
Kudos, SonarCloud Quality Gate passed! Quality Gate passed
Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells
No Coverage information No Coverage information
No Duplication information No Duplication information
Great work @2bndy5, merged.
I should draft a new version v1.4.0 to include the latest two enhancements.
Uh oh!
There was an error while loading. Please reload this page.
Use
Acceptarg in the header of therequestscall to get a list of changed files. If a token is supplied, then it is included in the header'sAuthorizationarg. This idea might be applicable to all REST API calls, but for security reasons (and I haven't verified with the REST API docs), it is only applied to theget_list_of_changed_files().(削除) I'm not confident, but I think (削除ここまで)this should address #38I have recently started using a VSCode extension that checks spelling. So, I also fixed some spelling errors in the python source code.