-
-
Notifications
You must be signed in to change notification settings - Fork 116
feat: connectivity view: quota for all transports #7630
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
base: main
Are you sure you want to change the base?
Conversation
and name the section Relay Capacity until we come up with a better name. and use email parse method to extract domain from email
src/quota.rs
Outdated
| assert!(t.quota_needs_update(TIMEOUT).await); | ||
| assert!(t.quota_needs_update(0, TIMEOUT).await); | ||
|
|
||
| // add entry and quota, check that it exists |
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.
Are these some todo comments?
Can be just removed probably.
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.
ah right forgot to add a test here, will do that
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.
There should be a test here that tests that the quota for a transport is removed, when that transport is deleted.
But I haven't found a simple way to add transports in rust tests at first glance and it is not too important, so I'll just remove the comments.
| let storage_on_domain = | ||
| escaper::encode_minimal(&stock_str::storage_on_domain(self, domain).await); | ||
| ret += &format!("<h3>{storage_on_domain}</h3><ul>"); | ||
| // TODO: stock string - when we decided on a good term, |
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 should be decided before we merge, otherwise this will be just a merged TODO.
Ideally something similar to existing applications, but I did not manage to find anything similar quickly.
Maybe something like "Inbox size", "Inbox space", "Inbox quota", "incoming message queue size", "message queue size" etc. I think "Inbox" already makes it clear that this is not a message archive, but something temporary, this is what we want to avoid someone thinking that relays store messages.
cc @r10s
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.
"Relay capacity" also looks good because it's "relay" and not "server", i.e. smth that retransfers messages, but doesn't store them indefinitely. But i'd lowercase "capacity", this text is just a description and not a UI control text
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.
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.
"Capacity" is similar to "Storage", and does not imply that it is temporary gets smaller and bigger as needed.
but there seems to be a term for exactly that: "Buffer" - it was discarded at #7580 (comment) as not being "good enough" - but it seems to describe better what it does, see eg. https://en.wikipedia.org/wiki/Data_buffer or in german https://de.wikipedia.org/wiki/Puffer_(Informatik)
if we agree that it is technically a "buffer", i would stay with that term (speaking of "chatmail" usage only here where things are deleted soon, we should not try to find a term that fits classic email usage as well)
"buffer" is also not that unknown - quite some user probably know that by various other techniques (netflix streaming buffers :) - and if ppl do not understand the term, they would probably also not understand "relay capacity" - but it would also not be too bad as the layout is also self-speaking
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.
technically it is a quota, an allowance of how much resources you can use at a given time or in a given time-frame. I'm still not convinced of the term "Buffer", at-least not without a small explanation sentence bellow the heading.
But In the end I don't really care. would you want "Relay Buffer" or just "Buffer"?
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.
well, quota is very much related to storage and capacity again, not implying things are temporarily. esp. when having more than one relay, ppl might thing, space sums up (they may still think that with "buffer" :)
i would just for "Buffer" - we're not introducing the term "Relay" much otherwise.
however, tbh, i would also not die on that hill - if "Buffer" does not work out or others already now find it weird as well, we can go for smth else. but at a first glance, i think it is a good start
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.
What about "Message Buffer Capacity"? ("Kapazität des Zwischenspeichers" in German)
- All of connectivity view is about relays, no need to mention "relay"
- It's about messages
- It's about the capacity for temporarily holding these messages
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.
bit long maybe, esp. for translations - german translation seems inaccurate, would be "Kapazität des Nachrichtenzwischenspeichers" or "Nachrichtenzwischenspeicherkapazität" (yeah, we're good in long words :)
so maybe shorten to "Message Buffers"? plural, as there are several buffers. that it is about "capacity and usage in megabytes" is clear by what follows (it is not only mainly capacity, thinking it over)
could look like the following (also flattened a bit):
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.
if we change the design, how would we show errors in a way that looks decent?
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.
in a similar way, so without headlines and additional indent. if in doubt, prefix them alway with host - same for other quota values - but both is rare and not the normal case
but as said, if easier, that can also be part of a subsequent PR, the layout in that area is no blocker for this PR
| let storage_on_domain = | ||
| escaper::encode_minimal(&stock_str::storage_on_domain(self, domain).await); | ||
| ret += &format!("<h3>{storage_on_domain}</h3><ul>"); | ||
| // TODO: stock string - when we decided on a good term, |
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.
"Relay capacity" also looks good because it's "relay" and not "server", i.e. smth that retransfers messages, but doesn't store them indefinitely. But i'd lowercase "capacity", this text is just a description and not a UI control text
|
ftr, i think, this eats up to much vertical space, degrading the other parts. however, this should not be a blocker and i'd also see that a bit in practise before iterating over that. all in all already lgtm, thanks a lot for taking care! ❤️ |

DC_STR_STORAGE_ON_DOMAINstock stringcloses #7591