It is still possible to turn off unattended flag in settings.yaml. This can lead to various bugs, because most of our development team never turn this off. Foreman has grown and with all the plugins, this is unnecessary extra work.
I propose to remove this flag which will simplify a lot of code.
$ ag 'SETTINGS\[:unattended\]' | wc -l
64
https://projects.theforeman.org/issues/23536
0 voters
I’d +1 but given the fact we’re receiving bugs, it means some users are using it. It would be good to know, how popular setting it is.
Big +1, I don’t think it makes sense given we don’t even test our software with :unattended => false. Pretty sure a lot of workflows will fail the moment you turn it off & we’re expected to respond to that.
I am not saying nobody is using that, but the burden is big and simplicity is sometimes the key. I think nobody will be annoyed if we explain the benefits to all of us - cleaner code, less bugs overall, faster tests.
The most important thing to understand before you vote is: by dropping this setting we are not dropping any feature. It’s just some people will see more flags in forms and some menu items and screens which were hidden for them. Not using them is a possibility, even permission system should be able to hide them if this is must.
It’s a pitty that we just published last year anniversary survey, but I don’t want to postpone this for another year, then we forget about this and month later you will read me crying here again.
By definition, that’s not true. You’re dropping the unattended feature, which also removes the ability to vastly simplify the UI, and the ability to have hosts with shortname-only (no domain), and probably other things besides. You can’t say “simplifies much of the codebase” and “no one will miss it, no features are lost” at the same time, these are incompatible statements.
I agree with Daniel that it’s a thing we don’t test, and it is frequently broken. But Marek is also correct. I’d like to see some code analysis of exactly what will be lost by this change, since it’s clearly not “nothing”. It’s possible that we’ll be OK with that loss, or that we will think of ways to reimplement some of the lost features, but that’s not possible until the consequences are clear.
Can you elaborate what exactly is on your mind here? Analysis of amount of dropped code or analysis of possible workarounds?
Permission system let’s you to remove whole menu entries. I tested this today, you can actually have menu with just one entry, this is very flexible system and it can be seen as replacement of unattended setting.
It does not allow users to simplify forms tho, going forward with this will make some forms more complex. I think the most important one is Host form.
We are working on Host form UX redesign, I think getting rid of unattended earlier might be a good chance to gather feedback from users. Maybe we need to implement some hide/show field UX feature if we find that this is the thing.
how about just making sure tests can run in unattended false? i’m pretty
sure a lot of tests can be skipped, but if its not a lot of effort to run a
test and keep it green, i would favor keeping the feature.
That was my thought when I said “code analysis” - I’m not suggesting detailed performance analyses, just what’s impacted and how much work it is to fix/maintain that. I think we all agree the current situation isn’t great, the choice is “support it properly” or “drop it”.
However, this is a core settings.yaml setting, so testing it requires a whole new run of the Jenkins test suite, probably on a lower frequency that the usual tests. Cloning the test_develop job in JJB and altering that setting in the setup seems feasible?
If we’re going to keep this feature, I suggest we spend time on refactoring this - https://github.com/theforeman/foreman/blob/develop/app/models/host/managed.rb#L205-L250
If condition that changes what methods are defined on object based on setting is IMHO the greatest source of problems. Conditions should move to those methods, which can be then tested separately. I wouldn’t run all tests with unattended mode disabled, but if we carefully look at where this setting apply, we could only add few tests for this mode that are missing.
Most of our tests run with disabled orchestration tho. I am not very confident in saying if tests are green the situation is better. And slower tests sound really bad to me.
Long-term goal of extracting provisioning from core would do it. But that’s something not on any table right now.
I am gonna dismiss my RFC, just four votes so far is not enough. There are reasonable concerns. Is there a list of things we want to put into 2019 survey? If not, shall we start a thread? We just need to make sure we won’t forget to review it beginning next year 
There isn’t a list, but I’ve made a note on my todo system
Let’s revisit this discussion - turns out that having unattended set to false broke fact importing in 2.2.0 (Bug #31364: undefined method `without_orchestration' - Foreman). What’s worse, is that we actually have a test that was supposed to verify that fact importing still works in that case. However, since the test environment gets loaded with unattended set to true and we only set it to false during the test setup, the host class already loads all the concerns and callbacks, leading to a false sense of security regarding these test paths (see https://github.com/theforeman/foreman/pull/8160#issuecomment-735418777 for details).
Considering that we are planning a Foreman 3.0 version soon, that might be a good opportunity to also drop this setting. I suggest that we mark it as deprecated in the upcoming 2.4 release and remove it in 3.0.
We can consider replacing it with a run-time setting that allows disabling orchestration if needed, but perhaps a better approach is to have orchestration depend on available capabilities and host configuration - e.g. dns step gets executed if a dns smart proxy is associated with the domain the host is assigned to etc. That means some currently required fields may be changed to optional, so that all that is needed to skip an orchestration step is not set the relevant field.
According to our survey, only 3.5 per cent of our users would be affected by this. How much time do we have until 3.0?
To bring this to users attention, I suggest to do one small change for 2.3 if it’s not too late - let’s somehow force the application into unattended=true mode and have some backup plan to revert this (e.g. via some configuration value or constant that can be edited). So people after upgrade would immediately notice this and come back to us if they have problems. If they hit some serious issues, we can tell them to disable the flag so we have some time to do some changes.
Oh now I realized this is not a regular setting, but the readonly setting. Therefore I suggest a setting/installer migration what would set this to TRUE and add some warning comment explaining what’s happening in 3.0.
IMHO the major problem with the unattended setting is that it changes the signature of classes and models. That is IMHO a bad design. The good part is that it can simplify the application for those who do not care about some aspects.
There’s a bunch of things that are already optional if empty. DNS is only executed if the DNS domain has a DNS Smart Proxy. I think this is a good design and would agree that more of that should happen.
That would mean it would be scoped to a simplification in the UI rather than in the entire application.
One possible solution would be to separate out some parts to a separate model. Then the unattended setting ensures the user is unable to create that model but all reading of it is unchanged.
Do you think it’s a good idea to hardcode the setting to “true” in the puppet/installer for the next major release (2.4) and plan removal for 2.5?
I have no idea how the database table structures are influenced by the setting and what other things. Will such a migration work? I’d guess that suddenly a lot of validations would start to fail. IMHO that needs to be worked out before we flip a switch.
Also, hardcoding it in a minor release is effectively dropping it. That is something you do on a major version. So 
to doing that in 2.4.
Turning it off or on “should just work”. Hardcoding is not the best way, probably “setting it to true as the default” is better. But we need to turn it on also for existing installations (upgrade) to get some feedback from the field.
I would suggest adding a notification in 2.4 for those who have it set to false indicating that we are planning to remove it in 3.0 and allowing them to manually change their setting or come back to us with what broke when they did.