-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-29787 Run Yetus General Checks as a GitHub Action (addendum) #7638
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
base: master
Are you sure you want to change the base?
Conversation
Conditionally execute publishing to nightlies.
|
🎊 +1 overall
This message was automatically generated. |
|
Context on https://issues.apache.org/jira/browse/INFRA-27571 . I'm not sure if this tweak will actually work without merging it, though maybe the PR from my external repo will teach us something. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Hmm strange. Nothing at all from GHA on #7639 |
|
How do other projects which use GHA store the build result? |
|
https://github.com/apache/dubbo/actions/runs/21034081969/job/60477885181?pr=16005 This project just uploads the artifacts to github. https://github.com/apache/dubbo/blob/3.3/.github/workflows/build-and-test-pr.yml |
|
OK, we also upload the yetus output to artifacts on github. So what do we want to upload to nightlies? Can we also upload them to github artifacts? |
|
I started by uploading to GH artifacts, not using nightlies. The trouble is with the links out from the report on the comment -- they need a url for each plugin's output. GH artifacts does not support this. With multiple artifact files (we ship everything from the build) it only permits you to upload an archive and there's no way to deep link into the contents of that archive. So, no links. I built out nightlies support thinking that would be a solution. To migrate to GHA, it appears that all failures need to be reported to the Checks API. We can use Annotations as well, but not all of our plugins support this (our spotless appears to be one such plugin). For all the plugins we want to run, we'll need to ensure that they report any details properly via annotation. Of course, a human can still download the entire test-patch archive bundle, but the need to know where to find it in the Actions run UI. |
|
Argh this means we don't have use of the hide-old-comments feature either. That will get annoying fast. I'm going to remove reliance on secrets in this addendum. Let's looks more closely at what remains. For hiding old comments, I guess we could implement a cron action. Let me add that and y'all can tell me what you think. And let me see if it's a simple thing to add linecomment support to our spotless extension. |
|
(!) A patch to the testing environment has been detected. |
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
|
💔 -1 overall
This message was automatically generated. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
OK, so the problem here is that, yetus has a I guess we need to check how does yetus render the table... |
e3af448 to
f63c617
Compare
|
(!) A patch to the testing environment has been detected. |
|
(!) A patch to the testing environment has been detected. |
1 similar comment
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
|
I had to re-implement the function that generates the comment, but this one looks better at least. Maybe this is all for nothing. The GHA isn't even writing a comment on #7639 . I think the compromise here is to extend Yetus to make each plugin's +1 as an explicit passing check. @Apache9 @petersomogyi WDYT? |
|
💔 -1 overall
This message was automatically generated. |
|
For me I think the current solution is acceptable, anyway I can download the artifacts and check the result. It will be better if we can add a line in the footer of yetus comment to point to the artifacts with a url(Can be a separated issue). And since now we can run general checks, it will be easier for us to also run the JDK17/11/8 hadoop2/3 checks too? In this way, we can remove the pre commit jenkins job then. Thanks. |
| @@ -1,25 +1,9 @@ | |||
| /* | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remember to revert the changes to this file before merging :)
|
I just left a note on the dev list to see if anyone else has other considerations. But yes, I think this is now working as designed -- no comment and only Checks. We can disable the Jenkins PR runner and start to explore converting other checks. Let me also remove this comment cleaning timer. |
|
💔 -1 overall
This message was automatically generated. |
Conditionally execute publishing to nightlies.