You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2022/03/24 08:33:48 UTC
[GitHub] [james-project] vttranlina opened a new pull request #936: JAMES-3729 LocalDelivery error handling is non-standard
vttranlina opened a new pull request #936:
URL: https://github.com/apache/james-project/pull/936
jira: https://issues.apache.org/jira/browse/JAMES-3729
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on a change in pull request #936: JAMES-3729 LocalDelivery error handling is non-standard
Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #936:
URL: https://github.com/apache/james-project/pull/936#discussion_r834072653
##########
File path: server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/LocalDelivery.java
##########
@@ -77,6 +77,8 @@ public void init() throws MessagingException {
.metric(metricFactory.generate(LOCAL_DELIVERED_MAILS_METRIC_NAME))
.build())
.consume(getInitParameter("consume", true))
+ .ignoreError(getInitParameter("ignoreError", false))
+ .errorProcessor(getInitParameter("errorProcessor", Mail.ERROR))
Review comment:
I'm not sure when onMailetException=propagate
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on a change in pull request #936: JAMES-3729 LocalDelivery error handling is non-standard
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on a change in pull request #936:
URL: https://github.com/apache/james-project/pull/936#discussion_r834221824
##########
File path: server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/delivery/MailDispatcher.java
##########
@@ -109,12 +101,12 @@ public MailDispatcher build() {
private final boolean ignoreError;
private final String errorProcessor;
- private MailDispatcher(MailStore mailStore, MailetContext mailetContext, boolean consume, boolean ignoreError, String errorProcessor) {
+ private MailDispatcher(MailStore mailStore, MailetContext mailetContext, boolean consume, String errorProcessor) {
this.mailStore = mailStore;
this.consume = consume;
this.mailetContext = mailetContext;
this.errorProcessor = errorProcessor;
- this.ignoreError = ignoreError;
+ this.ignoreError = errorProcessor.equalsIgnoreCase("ignore");
}
Review comment:
not sure if we need to customize the onMailetException handling 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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina removed a comment on pull request #936: JAMES-3729 LocalDelivery error handling is non-standard
Posted by GitBox <gi...@apache.org>.
vttranlina removed a comment on pull request #936:
URL: https://github.com/apache/james-project/pull/936#issuecomment-1077392674
I'm not sure when `onMailetException=propagate`
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #936: JAMES-3729 LocalDelivery error handling is non-standard
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #936:
URL: https://github.com/apache/james-project/pull/936#discussion_r834173327
##########
File path: server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/LocalDelivery.java
##########
@@ -77,6 +77,8 @@ public void init() throws MessagingException {
.metric(metricFactory.generate(LOCAL_DELIVERED_MAILS_METRIC_NAME))
.build())
.consume(getInitParameter("consume", true))
+ .ignoreError(getInitParameter("ignoreError", false))
+ .errorProcessor(getInitParameter("errorProcessor", Mail.ERROR))
Review comment:
You should throw an exception.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on pull request #936: JAMES-3729 LocalDelivery error handling is non-standard
Posted by GitBox <gi...@apache.org>.
vttranlina commented on pull request #936:
URL: https://github.com/apache/james-project/pull/936#issuecomment-1077392674
I'm not sure when `onMailetException=propagate`
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on a change in pull request #936: JAMES-3729 LocalDelivery error handling is non-standard
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on a change in pull request #936:
URL: https://github.com/apache/james-project/pull/936#discussion_r834150242
##########
File path: server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/delivery/MailDispatcher.java
##########
@@ -109,12 +101,12 @@ public MailDispatcher build() {
private final boolean ignoreError;
private final String errorProcessor;
- private MailDispatcher(MailStore mailStore, MailetContext mailetContext, boolean consume, boolean ignoreError, String errorProcessor) {
+ private MailDispatcher(MailStore mailStore, MailetContext mailetContext, boolean consume, String errorProcessor) {
this.mailStore = mailStore;
this.consume = consume;
this.mailetContext = mailetContext;
this.errorProcessor = errorProcessor;
- this.ignoreError = ignoreError;
+ this.ignoreError = errorProcessor.equalsIgnoreCase("ignore");
}
Review comment:
I think we can remove the ignore/propagate parsing. Our mailet pipeline already handles that. cf `ProcessorImpl`
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on pull request #936: JAMES-3729 LocalDelivery error handling is non-standard
Posted by GitBox <gi...@apache.org>.
vttranlina commented on pull request #936:
URL: https://github.com/apache/james-project/pull/936#issuecomment-1078790368
> Please add a test for the default behaviour when there is an error, if it is not tested yet ;-)
It's already `org.apache.james.transport.mailets.delivery.MailDispatcherTest#errorsShouldBeWellHandled`
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on a change in pull request #936: JAMES-3729 LocalDelivery error handling is non-standard
Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #936:
URL: https://github.com/apache/james-project/pull/936#discussion_r834903796
##########
File path: server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/delivery/MailDispatcher.java
##########
@@ -109,12 +101,12 @@ public MailDispatcher build() {
private final boolean ignoreError;
private final String errorProcessor;
- private MailDispatcher(MailStore mailStore, MailetContext mailetContext, boolean consume, boolean ignoreError, String errorProcessor) {
+ private MailDispatcher(MailStore mailStore, MailetContext mailetContext, boolean consume, String errorProcessor) {
this.mailStore = mailStore;
this.consume = consume;
this.mailetContext = mailetContext;
this.errorProcessor = errorProcessor;
- this.ignoreError = ignoreError;
+ this.ignoreError = errorProcessor.equalsIgnoreCase("ignore");
}
Review comment:
yes, it looks like a customized
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #936: JAMES-3729 LocalDelivery error handling is non-standard
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #936:
URL: https://github.com/apache/james-project/pull/936#discussion_r834051003
##########
File path: server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/LocalDelivery.java
##########
@@ -77,6 +77,8 @@ public void init() throws MessagingException {
.metric(metricFactory.generate(LOCAL_DELIVERED_MAILS_METRIC_NAME))
.build())
.consume(getInitParameter("consume", true))
+ .ignoreError(getInitParameter("ignoreError", false))
+ .errorProcessor(getInitParameter("errorProcessor", Mail.ERROR))
Review comment:
Please use the standard `onMailetException` property to do this.
CF https://james.staged.apache.org/james-distributed-app/3.7.0/configure/mailetcontainer.html#_error_handling
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa merged pull request #936: JAMES-3729 LocalDelivery error handling is non-standard
Posted by GitBox <gi...@apache.org>.
chibenwa merged pull request #936:
URL: https://github.com/apache/james-project/pull/936
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org