Now, it would be nice to give users ability to configure this value, which I am gonna propose to 100 by default. However we don’t want to add another setting.
I am wondering if we should create a file somewhere in /usr/share/foreman/config/constants.rb which would load /usr/share/foreman/config/constants.local.rb treated as ghost file (not overwritten during upgrade). Users would be able to fine-tune some of our internals. Documentation would be only in the constants.rb file, so good candidates for such values would be things we do not want to expose and let users play around however we want have the ability to tell them to fine tune some values in very special cases (biggest customers, special deployments, containers etc).
Something like this, typically never (or only once) changed by users having in file does not hurt. But I do not like configuration outside of /etc because it is typically overlooked by user and packager. We already have this problem with the ignored_environments.yaml.
I think this still counts as introducing another setting with the downside that it’s harder to configure, not visible and not possible to do any validations.
These are two main goals of my proposal - we don’t want regular users to be aware of these settings. Only in case of fire we (or manual) would provide instructions what to change and where.
It could be also harder to document, backup and debug when misconfigured. Not sure if it is worth it. Mostly I don’t touch the settings I don’t need but when I would need it I would prefer to find it in the Settings with other fact related settings accompanied by decent description.
Anyway among 50 other provisioning settings it will be best hidden from users .
However I feel limited, often in reviews I get a request not to introduce another setting. This 2nd tier of (internel) settings could be a nice way out.
Not sure what is the reason against adding more settings but if it is it’s too many of them I would prefer to discuss this with UX team and find some solution that scales better. E.g. display only the frequently used options and show everything only on demand? or add another level to the settings tree?
Adding more ways to configure Foreman does not improve the UX for me.
I believe @tbrisker is driving this. In general, I agree. We should not consider settings to be “free of charge”. However, it makes sense to have them so users can tune Foreman. So if it’s useful to add a new setting, we should not block a PR on this imho. I couldn’t agree more with what @mbacovsky says. This is a UX issue.
There are two reasons driving me to try and reduce the number of settings:
Reducing the complexity of Foreman to users. Foreman is a beast right now, and finding out what each of the >100 settings we have (just in core) does is difficult. Which of these should a user really modify? which might cause unexpected breakages? in many cases, because we weren’t opinionated enough on which way certain workflows should work, we just added a setting “so we can do both”, meaning our users get confused on how they should be using foreman.
Increasing stability and reducing the maintenance costs. The more settings we have, the more complex our code gets to handle these settings, and the more likely we are to hit bugs. We also can’t exhaustively test every possible setting combination, leading to paths that aren’t well tested and break more often.
Like @TimoGoebel said, settings aren’t “free of charge” - any trade-off in software development comes with its cost. I am not saying that we should completely avoid adding new settings (I actually merged a couple of new ones recently), but we should always consider if the trade-off is worth it and if we can be more opinionated and avoid adding a setting.
Regarding the original proposal - I would prefer not to add another file just to hold all “constants”. I would much prefer to settle on a sane default and keep it even as a “magic number” inside the code, at least until we hear from enough users that they want to change it - which to me means we didn’t pick the correct number, not that we have to make it customizable. Advanced enough users that will know to change the number in the file can also modify the code itself directly.
just decided to throw my two cents from a users perspective in here:
I would advise against any options that include adding more config files, especially if they were to be located in wierd locations like alongside the code. This will inevitable lead to more complex, harder to debug error scenarios. Foreman is complex enough as it is, and hiding more stuff in strange locations is not going to help.
Alongside 1. I would also recommend to not move stuff to hard to reach/use locations just to hide it from the users. By now, there are more than enough features in Foreman that I would consider highly undocumented and people in need of those tools are just going crazy searching for them.
Suggesting that “advanced users should patch the code” is the absolutely wrong way to go. The more this mentality gets around, the more users will face upgrade hell on a regular basis. In my opinion, you should not be required to patch the source of a program just to use it’s basic features. I have about 4 years of experience in installation, administration and maintenance of our Foreman instances, so I would consider myself an “advanced user”. But I am by no means a ruby dev, and every time I try to understand how things in the code work I end up banging my head against the wall. Patching the code yourself is not an option for most non-dev user.
Having “magic numbers” hardcoded in the sourcecode, arbitrarily selected via “I don’t think anyone needs more than this” is among the most annoying things you are faced as a user. If there is no technical limitation, do not limit it. Most users will not tell you the number is to low. They will assume there is reasoning behind that number and implement workarounds for that behaviour just to avoid hitting your arbitrarily selected “magic number”. Referencing the original proposal to drop fact hashes that are to big: We do have a custom hash-fact that has an avarage of about 1k child entries per host, and it is working just fine without noticable issues on our end. Having an immutable (except for patching) limit on this would inevitably force us to rewrite that fact in some nonsensical way that circumvents this issue.
Considering all of the above, I would generally recommend treating “dangerous” or “advanced” settings as a UI/UX issue as it has already been suggested. Hiding these behind some sort of “there be dragons” advanced settings button or something like that would be fine with me. Forcing users to jump through hoops just because the current solution might overwhelm newbies is not. While I agree Foreman needs a better “new user experience”, cutting or limiting features just because they might be complicated can not be the right solution.
Increased code complexity is indeed a valid reason to not make everything configurable, but breaking features for a certain amount of users without a technical reason is also kind of bad in my opinion.
I strongly prefer constants. In a single file intended for users to override is not a good one. Whether it makes sense for them to be concentrated in a single file I’m not sure.
I still keep the rule that I was taught in university: the only numbers you’re allowed to use inside code are 0, 1 and 2. All others should be constants describing their intention. It’s not a 100% rule since 60 * 60 * 24 is common enough to be descriptive in some cases.
Being opinionated can also be a bad thing. Let’s take Pulp and Katello for example. Katello is highly opinionated and the result is that we have quite a few users who actually built solutions very similar to Katello using pure Pulp. Once things get too opinionated, you’ll miss out on users. That’s why you always have to make the trade off between flexibility (with its increased maintenance burden) and rigidity (with its lower maintenance). I think that Foreman has initially gained a lot of adoption because it was very flexible (citation needed). We can gain a lot in the initial user experience but that doesn’t have to negatively impact advanced users.
Other than that, @areyus said it well and I have little to add.
Thanks you very much for your feedback @areyus! You convinced me.
I think there are several outcomes that can be agreed on (but please chime in if you disagree on any of these):
We need a way of hiding “advanced” settings so we don’t overwhelm novice users but allow tuning some specific options when needed. These settings should be IMHO all things that would not require modifying on 95% of use cases (assuming we manage to choose sane default values). We might need from some help from @Roxanne_Hoover or @terezanovotna regarding the user experience point of view and how it would be best to implement it (or if this goes against UX best practices).
All settings should be accessible from the UI/API if possible. The only exception should be the few that are needed during the app initialization, and perhaps these also should be exposed in the UI in some way that allows changing them for the next restart.
We should continue taking into consideration the cost of adding additional settings.
We should consider both from the user complexity point of view (e.g. “Will there be multiple users that need to modify this?”, “Does this lead to complicating the possible user workflows instead of defining best practice?”) and the code maintenance point of view (e.g. “Does this setting require a large amount of checks in multiple places?”. “Does this setting interact with other settings leading to a large combination matrix that is difficult to test?”).
Just because we can make something configurable doesn’t necessarily mean that we should, if we mange to find a better way.
We should also not default to adding settings for every value just because “maybe someone will want to modify this value at some point”, and wait for someone to actually ask for it. In this case, it’s fine to have a constant (fine, no magic numbers ) in the code. The exception is if this modifies existing behaviour in a way that may break existing workflows. Even if someone asks for changing some value, we should first ask ourselves if maybe we simply made a wrong assumption and the value should be changed for everyone instead of adding a setting.
We will always need to make the final decision on a case-by-case basis and weigh all considerations, but agreeing on some general guidelines would be good. Would also be good to get feedback from more people on the @core team on this matter.
I agree with all points you stated in this post, except for this one:
As mentioned, I am very much against arbitrary “magic numbers” (even when they are called constants ) unless they serve a technical or practical purpose. I assume you agree agree on this, but I still had the urge to emphasize this once more. Not adding a setting for every constant that has a reasoning behind it is probably still alright though, as long as it’s taken into consideration how likely it is that users have valid use-cases where they might need that number changed.
I did and first thing I did was I switched to expert level to see what’s there while I’m definitely not a kodi expert I wouldn’t inspire by hiding, I’d prefer to organize settings to advanced/expert categories and perhaps mark them with warnings.
The hiding is just an implementation detail. My suggestion was about introducing levels to settings so a beginner knows some settings can safely be ignored.
My first thought is colors which is probably going to be horrible. The same goes for icons (way too cluttered) if you have a lot of settings.
Another is sections but that might also be confusing. You may want a section on the tab configuration management that’s provisioning and within that section you have some beginner and some advanced fields.
I still like a slider on the top right to hide everything from a certain level and higher.
I’m hoping the @ui_ux team will have suggestions on what’s actually good UX.
I’m not sure if organizing fields by users expertness is a good step. It is pretty subjective and thus difficult to predict. Shame on me but Windows Registry editor and the tree structure of the settings there came to my mind. Adding one level could get decent number of related items to one page. Also some search/filtering may help.
.
) in the code. The exception is if this modifies existing behaviour in a way that may break existing workflows. Even if someone asks for changing some value, we should first ask ourselves if maybe we simply made a wrong assumption and the value should be changed for everyone instead of adding a setting.