Skip to content
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

intelmq.lib.bot.Bot.set_request_parameters doesn't honor defaults.conf #1594

Closed
gethvi opened this issue Aug 6, 2020 · 4 comments
Closed

Comments

@gethvi
Copy link
Contributor

gethvi commented Aug 6, 2020

The expected order of configuration:

  1. bot parameters from runtime.conf if present
  2. default values from defaults.confif present
  3. default values specified by the documentation or fail

This function sets it's own default values if bot parameters from runtime.conf are not present and doesn't use defaults.conf values.

@gethvi gethvi changed the title intelmq.lib.bot.Bot.set_request_parameters doesn't honor defaults.conf intelmq.lib.bot.Bot.set_request_parameters doesn't honor defaults.conf Aug 6, 2020
@ghost
Copy link

ghost commented Aug 19, 2020

The method uses self.parameters. In Bot.__init__, self.__load_defaults_configuration() is called, and later self.__load_runtime_configuration() overwriting the default's values. set_request_parameters then has fallback values, in case there's no value for a certain parameter given.

@ghost ghost added the component: core label Aug 19, 2020
@gethvi
Copy link
Contributor Author

gethvi commented Aug 28, 2020

@wagner-certat Thank you for the explanation. However it still feel unintuitive. I would like to suggest having a dedicated method utils.load_defaults_configuration(check: bool = True, path: str = DEFAULTS_CONF_FILE)
that loads defaults.conf, validates all the keys and if there is a key missing then it is either replaced by some "superdefault" value (e.g. mail_ssl: true ) or an exception is thrown.
Basically something like the part of intelmqctl.check that checks defaults.conf, but as a separate method and more fault proof.
With such approach there wouldn't be a need for any fallback values anywhere else ever again. Everything checked in one place.

@ghost
Copy link

ghost commented Sep 4, 2020

I don't think we can get rid of internal fallback defaults. Just think of the case that the configurations files are "unreachable" and the tools need to abort in a proper way including logging.

But: handling of internal fall-back parameters, the default configuration and higher levels of parameters (bot-level default values and runtime configuration) can / should be improved.
See #632, #644, #757, #1505, #1580

@ghost
Copy link

ghost commented Aug 20, 2021

create_request_session in utils uses the global parameters:

defaults = get_global_settings()

And the HTTPMixin as well of course: https://github.com/certtools/intelmq/blob/develop/intelmq/lib/mixins/http.py

-> Solved

@ghost ghost closed this as completed Aug 20, 2021
@ghost ghost self-assigned this Aug 20, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant