-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mailbox Quotas #1568
base: main
Are you sure you want to change the base?
Mailbox Quotas #1568
Conversation
* add quota column * modify users_query to return quota_rule
When the `maildirsize` file does not exist it causes the script to fail. The IOError is not caught by the execpt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work! This is very useful.
I do have some minor feedback, however.
And a question: Is it possible for users to view the usage percentage (in Roundcube)?
@@ -787,7 +788,7 @@ def get_latest_miab_version(): | |||
from socket import timeout | |||
|
|||
try: | |||
return re.search(b'TAG=(.*)', urlopen("https://mailinabox.email/setup.sh?ping=1", timeout=5).read()).group(1).decode("utf8") | |||
return re.search(b'TAG=(.*)', urlopen("https://raw.githubusercontent.com/jrsupplee/mailinabox/master/setup/bootstrap.sh", timeout=5).read()).group(1).decode("utf8") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not seem right.
@@ -805,11 +806,11 @@ def check_miab_version(env, output): | |||
latest_ver = get_latest_miab_version() | |||
|
|||
if this_ver == latest_ver: | |||
output.print_ok("Mail-in-a-Box is up to date. You are running version %s." % this_ver) | |||
output.print_ok("Mail-in-a-Box with quota support is up to date. You are running version %s." % this_ver) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not needed in the PR right? I understand this is useful when running your fork, but not in the 'official' MIAB.
elif latest_ver is None: | ||
output.print_error("Latest Mail-in-a-Box version could not be determined. You are running version %s." % this_ver) | ||
else: | ||
output.print_error("A new version of Mail-in-a-Box is available. You are running version %s. The latest version is %s. For upgrade instructions, see https://mailinabox.email. " | ||
output.print_error("A new version of Mail-in-a-Box is available. You are running version %s. The latest version is %s. For upgrade instructions, see https://github.com/jrsupplee/mailinabox/blob/master/README.md. " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also does not seem right.
@dexbleeker left some important comments that need to be addressed. We also never include full configuration files that clobber existing package configuration files. This PR adds three new configuration files that clobber existing ones. Those need to be removed and any settings changes they make to stock Dovecot configuration files would need to be made using our editconf tool or another method we use for other files we modify. |
Will quotas be integrated if these are addressed? |
Yes |
I can't promise to merge something I haven't seen, and I could have more requests, but I don't foresee a problem. |
The merge conflicts need to be resolved also. |
Which files are getting clobbered? If there are files that are copied it is because they did not previously exist in the installation. |
I understand that you need to see the final product, but it will be essentially what you see here with some updates. I stopped trying with the quotas pull request because you (@JoshData) made a remark that you were not sure that quotas should be a part of the master repository. So has a decision been made to integrate quotas or is it still under discussion? |
20-imap.conf and 90-quota.conf. 20-imap.conf is edited elsewhere in our setup so it can't also be clobbered. 90-quota.conf appears to be essentially empty (no non-comment lines) in the distribution-supplied file so clobbering may be ok, but to avoid apt upgrade conflict issues it may be better to create a new file.
I am OK to merge if the feedback is addressed, yes. In the years since this began people have used your PR and proven that it is a good solution - that's what changed. |
@jrsupplee any timeline on this one? it would be awesome! thanks so much. |
How can one help to get that merged ? Shall one fork the fork, address the feedback and...pull request to merge ? :/ The feedback that needs to be addressed is about 20-imap.conf that is "clobbered" ? How do we un-cloberred such ? |
@guizmoau I know when I run updates, if the update changes anything in |
Yes, that's what I'm referring to. |
Hello @JoshData , is there any change get quota working under official MiaB release? appreciate your working on that. thank you. |
I am not working on it. If someone completes this PR addressing my feedback, then I'll happily merge it. |
thank you per your reply. could you please inform us what we need to do to complete PR? |
Hello @jrsupplee and @dexbleeker I saw you have been working on this subject for some years ago and this PR still open. Is there anything I can help to complete this PR to Josh be able to merge it in to official release ? thank you |
Temperature check: if I were to pick this up, are we still interested in getting it merged? I can update it to the latest code and address changes etc. TBD on timeline. |
I read through the code in this PR, it seems pretty straight forward. The bootstrap script is weird, I agree, so I'll take a look at that. I'll make a new PR that includes these changes after I get the basic app running locally and try to understand what's changed in the main codebase over the last four years relative to this. Specifically on the last point, if anyone has any thoughts here please lmk. I can't see the conflicts for some reason on github, so it won't be till I pull it down and run the diff that I see the full extent of the challenge with updating this code to the latest version. |
There happened to be a discussion on the forum a few days back. Perhaps you saw it? In addition, the powerMiaB fork includes the quota branch, it might be of help if both branches have not diverged too much. |
I'm working with stylnchris, actually. Thank you for the links. I can take a look at PMiaB also, though I suspect my effort will be best placed looking at the current MiaB code and forward-porting the changes in this PR (plus the requested edits). I'd love to hear from @JoshData as getting this merged to main-line would be ideal. Either way, I'll make a new PR with the latest code from this repo mixed with the code from this branch and link it here when it's ready. I'll be picking this work up later this month. |
New PR here with conflicts resolved and updated to latest MiaB: All code review feedback addressed. A couple items I saw while working on it that I have questions about but otherwise should be ready for re-review |
This is an implementation of mailbox quotas using dovecot's quota capabilities. This pull request supersedes a previous pull request that contained some changes not pertinent to the main repository.