Control permissibility of driver config in extra from airflow.cfg

[dstandish]

I removed the hook param driver_in_extra i figured having it only in airflow settings was good enough but lemme know if you think we need both.

[potiuk]

LGTM. Though. Yeah @uranusjr was right that likely __ instead of _ is better.

[dstandish]

if you guys both feel that way after the following argument then i'll change it

• i agree that double under is better from readability perspective
• however, i think it's much worse from user discoverability perspective and i think that's most important
• i think as a user, when you are dealing with an invalid character, most intuitive would be that you replace it with a valid one -- not that you replace it with two valid ones.

what say you @potiuk @uranusjr

[potiuk]

I think two is more consistent with the idea behind. We don't really replace the character, we account for the fact that our config has no hierarchy of configurations. So one could argue that . represents subsection, not . in the section name. In ideal world we would have:

[providers]
    [odbc]
      config = value

And then __ would make perfect sense. But we can't because we are using the old ini format that does not have it.

But it has hit people back in the past even for regular sections. That's why in https://airflow.apache.org/docs/apache-airflow/stable/configurations-ref.html#elasticsearch-configs the env variables are explicitly spelled exactly as they should be, This is the "discoverabilty" that we are missing currently that I mentioned in the slack thread. And since then I can't recall single issue with using __ as separator between section and config.

I think if we care for users not making mistakes we should be very precise and add examples in the docs that they will be able to copy&paste - both for config and variable. And once we do it, decision whether to use __ or _ makes little difference.

(BTW. That does recall some of the discussions we had years ago when we wanted to add google provider specific config).

[jedcunningham]

Bad rebase here? nah, github problems.

I can see arguments for both _ and __. I think the strongest case for single is it keeps the section more clearly delimited. AIRFLOW__{section}__{config}. I lean single, but its not a strongly help opinion.

[dstandish]

Bad rebase here?

yeah apparently some github issue

I can see arguments for both _ and __. I think the strongest case for single is it keeps the section more clearly delimited. AIRFLOW__{section}__{config}. I lean single, but its not a strongly help opinion

nice so that makes two of us -- we're a deadlock

now if we can just cajole @ephraimbuddy to our side ...

[dstandish]

added a note in core howto/set-config to set env vars when dot present

[potiuk]

I am actually not super strong on that one (once the example is there - it does not matter). If anyone feels strong - go for whatever option you choose.

[eladkal]

i am not sure if we should have providers settings in Airflow core docs..
this may create issues if for some reason in future the provider will remove this.
This also goes against our goal of eventually extracting the provider code base out of airflow core (at least I think we want to be ready for a day where we might want to do it)

If we must do this I think it's preferred to add a note with a link to the relevant information in the provider docs (external link!) thus treating the providers doc as external resource like any other package.

[dstandish]

So... this is meant to be an example. If you like i can make it not refer to specific case. i thought about this.

[dstandish]

ok have a look please @eladkal