[GitHub] nifi issue #483: NIFI-1899 - Introduce ExtractEmailAttachments and ExtractEm...

[trixpan]

Github user trixpan commented on the issue:

https://github.com/apache/nifi/pull/483
  
@JPercivall Fixing the rebase. Please hold


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #483: NIFI-1899 - Introduce ExtractEmailAttachments and ExtractEm...

[JPercivall]

Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/483
  
Thank you for the complements. The "InterruptedException" essentially means 
something want this thread to stop for some reason (here's an overview of 
[this](http://www.yegor256.com/2015/10/20/interrupted-exception.html)). At that 
point, we just want to proceed without lingering or stopping as best we can. 
Also since we are synchronized on the message, when we resume we can be sure 
that we are the only ones working on the message at that time. So we just need 
to be sure the other thread does it's best to take into account what the state 
of the message is.

Also for future reference, you shouldn't rebase other people's commits into 
yours. In order to keep track of who did what (and who to blame when things 
break, lol) you should just use "git am ".


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #483: NIFI-1899 - Introduce ExtractEmailAttachments and ExtractEm...

[trixpan]

Github user trixpan commented on the issue:

https://github.com/apache/nifi/pull/483
  
@JPercivall Beautiful code! Loved it. Interesting to see your programming 
thinking. Great lesson (specially because I tried to achieve a similar - though 
more covoluted - logic previously and failed. heheheh)

It makes total sense how you replaced the latch with a timed wait + 
notifyAll.

The only question that I have is how do you deal with a spurious wakeup in 
this section:

```
 final long serverTimeout = 
context.getProperty(SMTP_TIMEOUT).asTimePeriod(TimeUnit.MILLISECONDS);
try {
message.wait(serverTimeout);
} catch (InterruptedException e) {
getLogger().info("Interrupted while waiting for Message 
Handler to acknowledge message.");
}
```

and this section:

```
try {
message.wait(serverTimeout - elapsed);
} catch (InterruptedException e) {
// Interrupted while waiting for the message to 
process. Will return error and request onTrigger to rollback
logger.trace("Interrupted while waiting for processor 
to process data. Returned error to SMTP client as precautionary measure");
```

Am I correct to assume you just preferred to ignore them and take a 
decision based on whatever information message object would have gathered by 
that time? Something like: 

if all completed by then, great, message gets acked, commited, client gets 
happy, but if message is partially processed (e.g. no returnCode when it hits 
the first wait above) test with resultCodeSetAndIsError and effectively treat 
it as success(commit) unless an error code is set?




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #483: NIFI-1899 - Introduce ExtractEmailAttachments and ExtractEm...

[JPercivall]

Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/483
  
@trixpan do you by chance have a template that I can use to validate the 
processors with?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #483: NIFI-1899 - Introduce ExtractEmailAttachments and ExtractEm...

[ijokarumawak]

Github user ijokarumawak commented on the issue:

https://github.com/apache/nifi/pull/483
  
@trixpan Exactly. "Having a single connection is enough" was my expected 
behavior, but it wasn't. The error doesn't happen all the time, but quit often, 
so I think it's a race condition. The related implementation is not NiFi 
codebase, it's in javax.mail library as client, and subethasmtp as SMTP server. 
I don't have better idea other than limit the acceptable configuration.

Wow, dead lock, Hope we can reach to a simple yet robust code in the end :-D




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #483: NIFI-1899 - Introduce ExtractEmailAttachments and ExtractEm...

[trixpan]

Github user trixpan commented on the issue:

https://github.com/apache/nifi/pull/483
  
@ijokarumawak - thanks for looking at it. 

So just for me to confirm:

PutEmail connects and tries to use SSL. This fails. It holds the socket

PutEmail connects again with plain text. This connection fails.

?

If so, isn't that the expected behavior of a single connection limit? The 
client shouldn't hold for a server if it intends to do another connection?

On another note it seems the synchronized section introduced a serious bug 
around concurrency. Seems like I created a deadlock. :-)

Looking at it at the moment.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #483: NIFI-1899 - Introduce ExtractEmailAttachments and ExtractEm...

[ijokarumawak]

Github user ijokarumawak commented on the issue:

https://github.com/apache/nifi/pull/483
  
@trixpan I am not sure 100% yet about the javax.mail.SocketFetcher's 
behavior, but I recommend to set higher value as default for `Maximum number of 
SMTP connection`, such as `1,000`, and probably validate it to be at least 2, 
to avoid getting `Too many connection` error. 
org.subethamail.smtp.server.SMTPServer's default is 1,000. Conservative users 
may set 1 and will encounter the issue..


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #483: NIFI-1899 - Introduce ExtractEmailAttachments and ExtractEm...

[trixpan]

Github user trixpan commented on the issue:

https://github.com/apache/nifi/pull/483
  
@ijokarumawak Still not clear to me how you are reaching the error.

Also, note the QUIT message is not a requirement, a client may chose to 
send more than a message per session, example:

```
   smtp_connection_reuse_count_limit (0)
  When  SMTP  connection  caching  is enabled, the number of 
times
  that an SMTP session may be reused before it is closed, or  
zero
  (no limit).
```
(Souce: http://www.postfix.org/lmtp.8.html)




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #483: NIFI-1899 - Introduce ExtractEmailAttachments and ExtractEm...

[ijokarumawak]

Github user ijokarumawak commented on the issue:

https://github.com/apache/nifi/pull/483
  
Looked at the SMTP server side, org.subethamail.smtp.server.SMTPServer to 
investigate how it uses maxConnections. It's used as a Semaphore. With 
successful transport, there's a QUIT message and server release the Semaphore. 
So next client can send message:

```
2016-07-11 12:09:58,794 D [org.subethamail.smtp.server.Session] Client: 
MAIL FROM:
2016-07-11 12:09:58,794 D [org.subethamail.smtp.server.Session] Server: 250 
Ok
2016-07-11 12:09:58,794 D [org.subethamail.smtp.server.Session] Client: 
RCPT TO:
2016-07-11 12:09:58,794 D [org.subethamail.smtp.server.Session] Server: 250 
Ok
2016-07-11 12:09:58,794 D [org.subethamail.smtp.server.Session] Client: DATA
2016-07-11 12:09:58,794 D [org.subethamail.smtp.server.Session] Server: 354 
End data with .
2016-07-11 12:09:58,795 D [org.subethamail.smtp.server.Session] Server: 250 
Ok
2016-07-11 12:09:58,795 D [org.subethamail.smtp.server.Session] Client: QUIT
2016-07-11 12:09:58,795 D [org.subethamail.smtp.server.Session] Server: 221 
Bye
```

But I noticed that NiFi tries to open new connection before the previous 
one is closed:

```
2016-07-11 12:05:16,151 D 
[org.subethamail.smtp.server.Session-/127.0.0.1:57774] SMTP connection from 
localhost/127.0.0.1, new connection count: 1
2016-07-11 12:05:16,151 D 
[org.subethamail.smtp.server.Session-/127.0.0.1:57774] Server: 220 127.0.0.1 
ESMTP Apache NiFi
2016-07-11 12:05:16,152 D 
[org.subethamail.smtp.server.Session-/127.0.0.1:57774] no more lines from client
2016-07-11 12:05:16,152 D 
[org.subethamail.smtp.server.Session-/127.0.0.1:57775] SMTP connection from 
localhost/127.0.0.1, new connection count: 2
2016-07-11 12:05:16,152 D 
[org.subethamail.smtp.server.Session-/127.0.0.1:57775] SMTP Too many 
connections!
2016-07-11 12:05:16,152 D 
[org.subethamail.smtp.server.Session-/127.0.0.1:57775] Server: 421 Too many 
connections, try again later
```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #483: NIFI-1899 - Introduce ExtractEmailAttachments and ExtractEm...

[JPercivall]

Github user JPercivall commented on the issue:

https://github.com/apache/nifi/pull/483
  
The email-nar does not have a LICENSE or NOTICE in its src directory.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---