| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
* Verify GETMAIL_POLL * Switch state-dir from /tmp/docker-mailserver/getmail to /var/lib/getmail * Add support for custom getmailrc_general.cf file * remove getmail 'message_log' option in favor of 'message_log_syslog' * Change config directory structure: ├── docker-data/dms/config │ ├── getmail-example.cf to ├── docker-data/dms/config │ ├── getmail │ │ ├── getmailrc_general.cf │ │ ├── imap-example.cf │ │ ├── pop3-example.cf
|
Since this introduces breaking changes and will need to be delayed until v15. Review is not a high priority for me right now (I've got a lot to juggle as-is). I will come back to this when I can, I still need to tackle an update to LDAP docs for 14.1 release before I can resume the active LDAP PR that brings breaking changes for v15 (and will require revised LDAP docs again). Presently wrapping up OAuth/OIDC related task for DMS docs. I should be able to provide an initial review here after I've got that out of the way 👍 At a quick glance, while I don't have an issue with introducing a supervisor service, what was the motivation to replace cron with a while sleep loop script? (we use cron for scheduling periodic actions elsewhere... because that's what it's meant for?) |
Sorry, something went wrong.
Is there a reason, not to release v15 next? I don't want this PR to be open for indefinite time (until v15 is eventually released) and handling merge conflicts in the meantime. I think we once discussed that: As soon as breaking changes are merged, the next version should be a major version. I see no Problem in skipping any 14.x releases / I see no value in forcefully releasing 14.x.
|
Sorry, something went wrong.
… example, which makes more sense compared to the container directories.
I don't have any reason. I just want to release with docs update before my LDAP PR is merged for a follow-up release for that. If you want to go straight to v15 that's fine by me and I'll review within the next few days. Reasons to replace cron job makes sense, thanks for that 👍 |
Sorry, something went wrong.
|
BTW: Anything more long-term that was considered for v15 can go into v16. Or we just wait with releasing v15. |
Sorry, something went wrong.
Getmail uses cron for notifications. If getmail fails to fetch emails from retriever, cron sends an error notification to the mailbox Will the supervisor solution send a notification in case of failure? The Fetchmail daemon does the same |
Sorry, something went wrong.
|
I thought about the same. However, DMS does not send any error mails to the user.
I cannot confirm that. When I change, e.g. the hostname to something not resolvable, the error appears in the mail.log, but no notification mail is sent: Aug 12 12:37:21 mail fetchmail[14032]: couldn't find canonical DNS name of does.not.exist (does.not.exist): Name or service not known
Aug 12 12:37:21 mail fetchmail[14032]: Query status=11 (DNS)
Aug 12 12:37:21 mail fetchmail[14032]: getaddrinfo("does.not.exist","pop3s") error: Name or service not known
Aug 12 12:37:21 mail fetchmail[14032]: POP3 connection to does.not.exist failed: Success
Aug 12 12:37:21 mail fetchmail[14032]: Query status=2 (SOCKET)
Why mail notification might not be a good idea / why I've decided against implementing something like that: Given the default poll interval of 5 minutes, you would receive an error mail every 5 minutes until the problem is resolved. |
Sorry, something went wrong.
I'm receiving such messages from fetchmail, maybe for authorization only: Fetchmail could not get mail from xxx. The attempt to get authorization failed. Since we have already succeeded in getting authorization for this connection, this is probably another failure mode (such as busy server) that fetchmail cannot distinguish because the server didn't send a useful error message. However, if you HAVE changed your account details since starting the fetchmail daemon, you need to stop the daemon, change your configuration of fetchmail, and then restart the daemon. The fetchmail daemon will continue running and attempt to connect at each cycle. No future notifications will be sent until service is restored. -- The Fetchmail Daemon
Agreed, this is an issue too |
Sorry, something went wrong.
Confirmed. When I configure a wrong password, then I also receive the same mail. |
Sorry, something went wrong.
There was a problem hiding this comment.
Apparently if I do the review in the diff view but choose to submit as "Comment" type with no top-level message, it silently updates the earlier review feedback without listing them under a review event for better visibility 🤔
So for reference, this is all that really remains:
Sorry, something went wrong.
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
getmail6 itself logs as "getmail" to syslog. we align here to "getmail" too.
|
I've checked the boxes in your post, that I consider to be done. I've commented the other ones and waiting for your final feedback there 👍
I noticed some glitches? too. For example, when I reply in the diff view, I can't see my answer here in the conversation tab. I just see your comment but not my reply here. |
Sorry, something went wrong.
logger --id=$$ ... leads to this error message
logger: send message failed: Operation not permitted
The same issue is mentioned here: jonathanio/update-systemd-resolved#25
While I agree, logger was a little unexpected, as I noted in that comment you responded to 😝 I'm fine with git blame as sufficient for now. I was going to suggest adding a clarifying comment for the $$ / ${$} bash syntax to acquire the script PID, since for anyone unfamiliar that is not search friendly 😂 Interesting that it caused an error 🤔
Yeah, that's related to what I was saying... kinda. If I do a review in the diff view tab and submit the review with feedback added to existing review threads, I think it only shows my new review feedback comment for visibility (sadly with no link to the earlier thread it belongs to). The actual discussion thread remains attached to the previous review that introduced the review comment. Although it may no longer be visible in the diff view tab if the original lines it referenced were since deleted (or modified too heavily?). So most of the time it's easier to browse the diff view tab for an overview as the conversation tab gets unwieldly (unresolved comment threads also can be hidden under the "load more" button). When there's too many threads littered around to navigate I make an overview comment with links so it's easier for us both 😅 |
Sorry, something went wrong.
There was a problem hiding this comment.
Cheers for putting the time into improving this! Great work❤️
Sorry, something went wrong.
There was a problem hiding this comment.
Just small questions and nitpicks - overall a good PR :)
Sorry, something went wrong.
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
|
Documentation preview for this PR is ready! 🎉 Built with commit: f48aa22 |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Description
Related discussion: #4149
New:
Changed:
Breaking Changes:
Not covered by this PR / ToDo:
Type of change
Checklist