Make DEFAULT_OS_SEARCH_QUERY configurable

[dosas]

Problem Statement

DEFAULT_OS_SEARCH_QUERY is not configurable via settings

Solution

Make it configurable via settings

Tests to run

tests/foreman/api/test_provisioning_puppet.py::test_positive_puppet_bootstrap

[Satellite-QE]

Can one of the admins verify this patch?

[lhellebr]

I think this is OK but somewhat inconsistent with a lot of other constants that also should be a setting but are constants. And is server.yaml the right place? Any thoughts on this? Meanwhile, I will run PRT.

[lhellebr]

This means that the parameter is forbidden from existing and the run will fail in any case (because if it isn't in the config file, we also provide default value):

dynaconf.validator.ValidationError: server.default_os_search_query cannot exists in env main

[lhellebr]

My comment for must_exist is a blocker.

[lhellebr]

@dosas , now you require that parameter in config. I think what you originally wanted to do was to make it optional with default value (and I think that's a good approach). If I understand Dynaconf correctly, you can achieve that by not specifying any value to must_exist

[dosas]

@dosas , now you require that parameter in config. I think what you originally wanted to do was to make it optional with default value (and I think that's a good approach). If I understand Dynaconf correctly, you can achieve that by not specifying any value to must_exist

Thanks.

[dosas]

@lhellebr Is there anything left for approval?

[lhellebr]

@dosas I have started a PRT job for you.
Also, I'm giving you my ACK code-wise (i.e., pending green PRT results).
I will be out of office for a few days so someone else will have to pick this up - they'd have to anyway, this needs at least one more reviewer.

[lhellebr]

Actually, adding one comment where I don't like hardcoded versions so ACK pending PRT and that comment.

[lhellebr]

Why major="6" OR major="7" OR major="8" OR major="9"? What about 10? What about potential future versions? Why for 6..9?

[ogajduse]

Are we sure we want to open this can of worms? #10481

@dosas is simply moving the search string from constants to config.

If I understand what all those fixtures are trying to do, it is that they're trying to retrieve an operatingsystem entity from the API - the same type, family, and version that the server is installed on. Correct?

If I were to suggest a solution, I'd say that sat_object.os_version.major could help us identify the major version of the OS, and we could base the search string on that.

Or could we use the default_os fixture instead of these custom fixtures that are being modified as a part of this PR?

[lhellebr]

And here