You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/10/21 23:12:40 UTC
[GitHub] [activemq-artemis] somdatta8172 opened a new pull request #3809: Register listener
somdatta8172 opened a new pull request #3809:
URL: https://github.com/apache/activemq-artemis/pull/3809
Issue Link: https://issues.apache.org/jira/browse/ARTEMIS-3085?filter=12350956
Hello @clebertsuconic
I have added registerIOCriticalErrorListener method to ActiveMQServer Interface,wrote the method's implementation and added IOCriticalErrorListenerTest also,though need suggestions on it.
Thank you
Somdatta
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] nbrendah edited a comment on pull request #3809: Register listener
Posted by GitBox <gi...@apache.org>.
nbrendah edited a comment on pull request #3809:
URL: https://github.com/apache/activemq-artemis/pull/3809#issuecomment-949616073
> You should probably squash all your changes Into a single commit.
@somdatta8172 The workflow i personally use. If its wrong, then, it will be an honor seeing recommendations
```
git pull --rebase origin <branch_name>
git log --oneline # To confirm that the commits are present
git rebase -i HEAD~6 # number of commits you have on your branch on upstream
git push -f origin <branch_name>
````
replace `pick` with `squash` from bottom to top, leaving out the first. If you feel like deleting some commit, the replace `pick` with `drop`
Best wishes this
Brendah
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] nbrendah edited a comment on pull request #3809: Register listener
Posted by GitBox <gi...@apache.org>.
nbrendah edited a comment on pull request #3809:
URL: https://github.com/apache/activemq-artemis/pull/3809#issuecomment-949616073
> You should probably squash all your changes Into a single commit.
@somdatta8172 The workflow i personally use. If its wrong, then, it will be an honor seeing recommendations
```
git pull --rebase origin <branch_name> # get all commits pushed on <branch_name> locally
git log --oneline # To confirm that the commits are present
git rebase -i HEAD~6 # number of commits you have on your branch on upstream
````
replace `pick` with `squash` from bottom to top, leaving out the first. If you feel like deleting some commit, the replace `pick` with `drop`
After a message `rebase successful`, or its equivalent,
git push -f origin <branch_name>`
Best wishes this
Brendah
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3809: Register listener
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3809:
URL: https://github.com/apache/activemq-artemis/pull/3809#issuecomment-949255376
You should probably squash all your changes Into a single commit.
Also, you are adding the lister to a collection on ActiveMQServer, but you're not doing anything with it. you should register it to the StorageManager when it starts...
ultimately you should just delegate it from the storage manager.
Also: notice the critical error is meant for IO Storage.. not acceptors.. on your test you did an invalid TCP operation, but that wouldn't cause any damage.
We have some tests that are validating the IOCriticalException by simply calling an artificial method, generating a fake failure:
ShutdownOnCriticalIOErrorMoveNextTest was doing that, but that would be a bit convoluted for your test.
As for the change, you will have to change ActiveMQServerImpl a bit further.
Currently the ActiveMQServerImpl is creating a DefaultCriticalErrorListener that will register into the StorageManagers.
I suggest you add this ioCriticalErrorListener to your collection,
and then create a new Listener that will do the following:
ioCriticalErrorListeners.forEach((t) -> t.callitWhateverTheMethodName);
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] nbrendah commented on pull request #3809: Register listener
Posted by GitBox <gi...@apache.org>.
nbrendah commented on pull request #3809:
URL: https://github.com/apache/activemq-artemis/pull/3809#issuecomment-950193923
> git pull --rebase origin <branch_name>
Ooohh yes @clebertsuconic, you know some times we just edit files and create commits using github instead of using local git.
I use the previous command to get such commits **"updated ........"**
I think the equivalent command is `git fetch --all ` honestly, i am not so sure. Am not a Linus Torvalds :laughing:
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] nbrendah edited a comment on pull request #3809: Register listener
Posted by GitBox <gi...@apache.org>.
nbrendah edited a comment on pull request #3809:
URL: https://github.com/apache/activemq-artemis/pull/3809#issuecomment-950193923
> git pull --rebase origin <branch_name>
Ooohh yes @clebertsuconic, this is a weird hack. You know some times we just edit files and create commits using github instead of using local git. Such commit are on Githib and not the local environment.
I use the previous command to get such commits **"updated ........"**
I think the equivalent command is `git fetch --all ` honestly, i am not so sure. Am not a Linus Torvalds :laughing:
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] nbrendah commented on pull request #3809: Register listener
Posted by GitBox <gi...@apache.org>.
nbrendah commented on pull request #3809:
URL: https://github.com/apache/activemq-artemis/pull/3809#issuecomment-949616073
> You should probably squash all your changes Into a single commit.
@somdatta8172 The workflow i personally use. If its wrong, then, it will be an honor seeing recommendations
```
git pull --rebase origin <branch_name>
git log --oneline # To confirm that the commits are present
git rebase -i HEAD~6 # number of commits you have on your branch on upstream
git push -f origin <branch_name>
````
replace `pick` with `squash` from bottom to top, leaving out the first.
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] nbrendah edited a comment on pull request #3809: Register listener
Posted by GitBox <gi...@apache.org>.
nbrendah edited a comment on pull request #3809:
URL: https://github.com/apache/activemq-artemis/pull/3809#issuecomment-949616073
> You should probably squash all your changes Into a single commit.
@somdatta8172 The workflow i personally use. If its wrong, then, it will be an honor seeing recommendations
```
git pull --rebase origin <branch_name> # get all commits pushed on <branch_name> locally
git log --oneline # To confirm that the commits are present
git rebase -i HEAD~6 # number of commits you have on your branch on upstream
````
replace `pick` with `squash` from bottom to top, leaving out the first. If you feel like deleting some commit, the replace `pick` with `drop`
After a message `rebase successful`, or its equivalent,
`git push -f origin <branch_name>`
Best wishes this
Brendah
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3809: Register listener
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3809:
URL: https://github.com/apache/activemq-artemis/pull/3809#issuecomment-949251165
Please.. rebase against main:
```
git remote add github https://github.com/apache/activemq-artemis.git
git pull --rebase github main
git push origin register-listener -f
```
a merge commit on a pull request is never good.. you should always have your pull request against main. and if you update against main, please rebase and push -f
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] nbrendah edited a comment on pull request #3809: Register listener
Posted by GitBox <gi...@apache.org>.
nbrendah edited a comment on pull request #3809:
URL: https://github.com/apache/activemq-artemis/pull/3809#issuecomment-950193923
> git pull --rebase origin <branch_name>
Ooohh yes @clebertsuconic, this is a weird hack. You know some times we just edit files and create commits using github instead of using local git.
I use the previous command to get such commits **"updated ........"**
I think the equivalent command is `git fetch --all ` honestly, i am not so sure. Am not a Linus Torvalds :laughing:
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3809: Register listener
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3809:
URL: https://github.com/apache/activemq-artemis/pull/3809#issuecomment-949265138
remember to build with Checkstyle before updating the PR:
```
mvn -DskipTests=true -Pdev -Derrorprone
```
The -Pdev will enable checkstyle checks
and the errorprone will be used also if you set the variable on maven.
I like to build without tests (although I recommend running your tests before uploading it).
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3809: Register listener
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3809:
URL: https://github.com/apache/activemq-artemis/pull/3809#issuecomment-949831081
@somdatta8172 you still have a merge commit here.. I did the rebase myself and the merge commit disappeared...
also, did you see my points about the other changes?
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic edited a comment on pull request #3809: Register listener
Posted by GitBox <gi...@apache.org>.
clebertsuconic edited a comment on pull request #3809:
URL: https://github.com/apache/activemq-artemis/pull/3809#issuecomment-949251165
Please.. rebase against main:
```
# some people call it upstream
git remote add github https://github.com/apache/activemq-artemis.git
git pull --rebase github main
git push origin register-listener -f
```
a merge commit on a pull request is never good.. you should always have your pull request against main. and if you update against main, please rebase and push -f
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] nbrendah edited a comment on pull request #3809: Register listener
Posted by GitBox <gi...@apache.org>.
nbrendah edited a comment on pull request #3809:
URL: https://github.com/apache/activemq-artemis/pull/3809#issuecomment-949616073
> You should probably squash all your changes Into a single commit.
@somdatta8172 The workflow i personally use. If its wrong, then, it will be an honor seeing recommendations
```
git pull --rebase origin <branch_name> # get all commits pushed on <branch_name> locally
git log --oneline # To confirm that the commits are present
git rebase -i HEAD~6 # number of commits you have on your branch on upstream
git push -f origin <branch_name>
````
replace `pick` with `squash` from bottom to top, leaving out the first. If you feel like deleting some commit, the replace `pick` with `drop`
Best wishes this
Brendah
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3809: Register listener
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3809:
URL: https://github.com/apache/activemq-artemis/pull/3809#issuecomment-949831714
you rebased against your origin here:
git pull --rebase origin <branch_name>
you have to rebase against upstream/main
git pull --rebase upstream main
git push origin -f <branch-name>
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] nbrendah edited a comment on pull request #3809: Register listener
Posted by GitBox <gi...@apache.org>.
nbrendah edited a comment on pull request #3809:
URL: https://github.com/apache/activemq-artemis/pull/3809#issuecomment-950193923
> git pull --rebase origin <branch_name>
Ooohh yes @clebertsuconic, this is a weird hack. You know some times we just edit files and create commits using github instead of using local git. Such commit are on Githib and not the local environment.
I use the previous command to get such commits **"updated FILE_NAME ........"** (sometimes the user changes commit messages)
I think the equivalent command is `git fetch --all ` honestly, i am not so sure. Am not a Linus Torvalds :laughing:
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org