[GitHub] [hbase] bbeaudreault commented on pull request #4770: HBASE-27238 Backport backup restore to 2.x
bbeaudreault commented on PR #4770: URL: https://github.com/apache/hbase/pull/4770#issuecomment-1399111088 > @bbeaudreault How to re run the CI pipeline? Only committers can do it. Actually I was going to merge anyway, cuz we got 3 +1's which is all we really need. The pipeline failed on the last reporting step, unrelated. But now just the few more comments above -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on pull request #4770: HBASE-27238 Backport backup restore to 2.x
bbeaudreault commented on PR #4770: URL: https://github.com/apache/hbase/pull/4770#issuecomment-1398400263 There are some javac (checkstyle) warnings. They are all in the net-new code, and I checked them and master has the same issues. Seems like we skipped cleaning up checkstyle in backup/restore prior to now. Let's ignore those failures, I think I will create a separate jira to do checkstyle cleanup of `hbase-backup` module in master and branch-2 once this is merged. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on pull request #4770: HBASE-27238 Backport backup restore to 2.x
bbeaudreault commented on PR #4770: URL: https://github.com/apache/hbase/pull/4770#issuecomment-1398394580 Nice! I think this looks good. Just waiting on clean pre-commit checks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on pull request #4770: HBASE-27238 Backport backup restore to 2.x
bbeaudreault commented on PR #4770: URL: https://github.com/apache/hbase/pull/4770#issuecomment-1397708589 Thanks for doing this @rda3mon -- i made a few small comments, and then I think we should be good. Extra reminder: make sure to run `mvn spotless:apply` after making the requested changes here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on pull request #4770: HBASE-27238 Backport backup restore to 2.x
bbeaudreault commented on PR #4770:
URL: https://github.com/apache/hbase/pull/4770#issuecomment-1315969416
May still be a bit, but I'm finally starting to look at this now. Looks like
it's almost all net-new code.
I checked out this branch and the following:
```
git diff --color --name-only --diff-filter=A branch-2
HBASE-27238-backport-backup-restore-to-2.x | xargs -I{} git diff master -- {}
```
Auditing the output, it shows that all of the net-new files here are
relatively identical to the files from `master`. There are a few discrepancies,
but all related to HBaseTestingUtility vs HBaseTestingUtil (an unfortunate
difference between master/branch-2).
The files with actual changes fall into four categories:
1. pom.xmls -- all seem basic boilerplate
2. hbase shell change -- just adds support for backup/restore, looks good
3. java files in test directories
4. java files in source directories
There are only 3 source directory java file changes, and 2 in test. I'll be
focusing my efforts on those.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on pull request #4770: HBASE-27238 Backport backup restore to 2.x
bbeaudreault commented on PR #4770: URL: https://github.com/apache/hbase/pull/4770#issuecomment-1253137587 Sorry for the delay here. We are currently evaluating internally in our fork. Will get to reviewing in the near future. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on pull request #4770: HBASE-27238 Backport backup restore to 2.x
bbeaudreault commented on PR #4770: URL: https://github.com/apache/hbase/pull/4770#issuecomment-1245477406 @rda3mon looks like that cherry-pick resolved most of the error prone issues. Looks like there are 3 left: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4770/5/artifact/yetus-general-check/output/diff-compile-javac-root.txt They look pretty simple/javadoc related. would you mind pushing 1 more commit to fix those. otherwise the build looks clean; the test failure looks unrelated. once you push that fix, i can re-run the tests if needed. Gonna start reviewing the code this week hopefully. Can you link me to specific areas of this PR that had merge conflicts or net-new code, if possible. I know in slack you mentioned WALPlayer; I'll take a look at that. Any others would be helpful -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on pull request #4770: HBASE-27238 Backport backup restore to 2.x
bbeaudreault commented on PR #4770: URL: https://github.com/apache/hbase/pull/4770#issuecomment-1244797927 @rda3mon I find that the violations are usually pretty straightforward. If there are any that you have a question about, feel free to post here. I do think it's possible to run locally, maybe with `-PerrorProne`. But actually, you could try including https://github.com/apache/hbase/commit/2c3abae18aa35e2693b64b143316817d4569d0c3 in your back port here. Seems like Andrew already handled these in master. This has me wondering if there are other patches we need to include though. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on pull request #4770: HBASE-27238 Backport backup restore to 2.x
bbeaudreault commented on PR #4770: URL: https://github.com/apache/hbase/pull/4770#issuecomment-1244458268 @rda3mon I know this is still just a draft, but can you address the error-prone findings in the pre-commit? Shows up under javac: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4770/3/artifact/yetus-general-check/output/diff-compile-javac-root.txt Can you do it in a separate commit so that I can review it separately from the initial backport? I re-ran the pre-commit hooks and all the tests are passing, the above is the only automated failure test remaining. The spotbugs error is a known issue which I think will be fixed in https://issues.apache.org/jira/browse/HBASE-27363 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
