Luzifer
feat: human readable options, and a sane default
add expiryChoicesHuman, remove 'default expiry' from dropdown
Focussing on front-end UI, 'default expiry' means nothing. Rather than relying on non-human readable, unix timestamps, introducing human readable options: (s)econds, (m)inutes, (h)ours, (d)ays. The defaultExpiryHuman should be any of the options for expiryChoicesHuman. If it's not, it'll fallback to null, which will fallback to the defaultExpiry behaviour as it is now
Save the selected option as a cookie, only after initially loaded. Load from cookie on page load. API is not changed, still accepts seconds
I've seen a (few?) discussions on the default expiry being confusing, and I concur. This is my attempt to deal with that. I can't oversee the changes required for cli tool. I also am very aware that this change is backwards compatible, but picking this over the other introduces a breaking change.
I've chaos-monkey tested this a lot: I've not been able to break this, by removing the cookie, setting an illegal value, etc.
Let me know what you think!
Hm, why add new configuration options and basically duplicate the code for expiries?
The customizations are only set by a systems operator, and there is already a translation from seconds to human-readable in the frontend. Now we do have code to translate from seconds to human-readable and code to translate from human-readable to seconds and two backend options basically doing the same. That's too confusing… From an ops perspective, I have these questions in my head: Which one do I set now? Why are there two? Don't they do the same?
If you just want to have the expiry choices in a human way, you could just use []time.Duration which in the backend will automatically get parsed (of course, only with seconds, minutes, or hours but not days) but that would be a breaking change. I'm not sure whether a breaking change for a backend configuration to be human-readable can be justified.
I'm in for just selecting the first of the given choices and dropping the "default" option if it causes that much trouble. Likewise, I also like storing the choice, but please use LocalStorage instead of cookies (see the set-color-scheme key for storing the color scheme - of course this can be done in the component and does not have to be done in the index.html).
For the size formatting: Did you test JS doesn't do stupid stuff like Python creating 1.000000000000000001 with that change? Also, I'm not sure about this: 64.0 Mi states "it's point-zero," while 64 Mi could also be 64.49 Mi from a reader perspective… …not sure whether that's something "normal" people (so other than me) assume… 🤔 Personally, I don't like precision values to "jump" (so sometimes have decimal places, sometimes not).
I should have clarified: rather than see this as a feature-complete ready-to-merge PR, this was my suggestion to streamline the setup and admin-side of things, with full support for the human readable format: as a working end product
Are you open to the human-readable setup format rather than unix timestamps? If so, I reckon I could add support for both in the same config setting (all it would change is int[] to string[] in the schema, not sure if that is backwards incompatible?).
I'm in for just selecting the first of the given choices and dropping the "default" option if it causes that much trouble
I'd still prefer being able to specify a certain default, rather than the first one. Just gives the implementing party more freedom to do as they want, without it being too much of a bother maintenance wise.
re: cookies: I'll swap the cookie setup for localStorage, absolutely.
not sure whether that's something "normal" people (so other than me) assume… 🤔
Hahah, I reckon not a lot of people even use MiB rather than the plain old MB. The world is already messed up with hardware vendors selling 500 GB drives, and Windows interpreting that as ~465 GB. Personally, I'd prefer to see 64 MB or 40.5 MB even, rather than 64 MiB (=67.something MB), as that is something everyone should get.
In my case, I've used this service before to share secrets with clients, and even as they'd barely be able to make a distinction between a mouse and a keyboard, they would know what a megabyte is, and how that works.
I think the most simple implementation would be to switch ExpiryChoices from []int64 to []time.Duration. In YAML you then can specify i.e. 168h, 1h, 1s, … - We just need to test if it then accepts expiryChoices: [3600] or fails as that's a number and it expects strings (not sure how the unmarshal-logic handles plain int).
We then need to catch someone having specified i.e. 3600 for one hour but that's simple: Loop through the durations and if its x < time.Second just multiply with time.Second (as Go measures in nanosecond we would make the mistake not to multiply if someone specified more than 31.69 years of retention which is quite unrealistic).
Also before delivering the times to the frontend it needs to be converted back to []int64 by int64(x / time.Second).
That way the whole "human to seconds" logic can be removed as we do have seconds to human while the existing logic is still used. And the breaking change shouldn't be that big. Together with #222 we could make the jump to v2.0…