You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/09/28 19:21:15 UTC
[GitHub] [airflow] fritz-astronomer opened a new issue #18582: Allow Pools to be set via ENV
fritz-astronomer opened a new issue #18582:
URL: https://github.com/apache/airflow/issues/18582
### Description
As a system/airflow admin - I prefer being able to see all the various configurations of my instance, and being able to version and audit them.
Pools are a very important subsystem in Airflow - however they are currently only configurable via the UI and CLI.
I would like to be able to configure Pools via ENV variables, as I am able to with configuration settings, connections, and variables.
Something like `AIRFLOW_POOL_DEFAULT_SIZE = 128`
### Use case/motivation
_No response_
### Related issues
_No response_
### Are you willing to submit a PR?
- [ ] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] fritz-astronomer edited a comment on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
fritz-astronomer edited a comment on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-931461737
I'd imagine that'd be it - @SamWheating . Create and update upon startup, would be enough for most use-cases. I imagine it would need to be semantically the same as with Vars / Connections set via the ENV (read-only, unable to modify? I've never actually tried to modify a Var that was set via the ENV)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] BasPH commented on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
BasPH commented on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-935766452
Yes; I always liked this hot reload functionality of Prometheus: https://prometheus.io/docs/prometheus/latest/configuration/configuration/. +1 for rewriting the configuration, it has other nasty problems e.g. https://github.com/apache/airflow/issues/8255
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-934635333
> @potiuk (thinking out loud) I think there are many ways the user experience around configuration could improve. E.g. connections and variables set via env vars could be listed in the UI and we could periodically sync configuration to e.g. find discrepancies in configuration between instances. Several things in Airflow are already import-able using a json file. I don't have great experience with terraform state, but how about we extend the importing functionality to make all things import-able from one single json file? The config sync could be another API endpoint?
Sounds like a good idea, especially that it might actually open up possibility for something much more interesting (not sure if we want that) - namely reloading configuration provided outside of the DB without restarting Airflow. It would not be possible for all parameters, but I think that might be a good opportunity to refactor the "conf" part and make it truly independent from other parts of Airflow. Right now we have those nasty loops `conf -> models -> settings -> conf` which are creating all kinds of problems (for example cyclic imports when initializing secret backends). I'd very much support rewriting this part.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-932874697
Go ahead!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] BasPH commented on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
BasPH commented on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-933792473
@fritz-astronomer FWIW pools can also be imported from a json file: https://stackoverflow.com/a/69255203/3066428
IMO if we support configuration via env vars, we should either support everything or nothing, so I'd opt for having pools configurable via env vars since we support env vars for everything else too.
@potiuk (thinking out loud) I think there are many ways the user experience around configuration could improve. E.g. connections and variables set via env vars could be listed in the UI and we could periodically sync configuration to e.g. find discrepancies in configuration between instances. Several things in Airflow are already import-able using a json file. I don't have great experience with terraform state, but how about we extend the importing functionality to make all things import-able from one single json file? The config sync could be another API endpoint?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
eladkal commented on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-1035977187
PR: https://github.com/apache/airflow/pull/21223
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-1040876512
Adding whole functionality of syncing env/state/db in case it can be replaced with
```
airlfow pools import <file>
```
is a hard no for me.,
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk closed issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
potiuk closed issue #18582:
URL: https://github.com/apache/airflow/issues/18582
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] fritz-astronomer commented on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
fritz-astronomer commented on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-942718143
Yea @thcidale0808 - I imagine that would fit the needs of the individuals I've talked to with this. The ENV does it's best to ADD or UPDATE pools. Users will be made aware that pools (like connections and variables) can also come from many sources, and if the env disappears, then it's on the user to remove the pool (which also doesn't feel like the worst "failure state" to me?)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SamWheating edited a comment on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
SamWheating edited a comment on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-931473731
So we _can_ modify an env var, but the issue is that these changes aren't persisted anywhere and will be wiped or reset on a server or container restart, hence its not really a reliable place for long-lived read-write values.
Variables, connections and secrets set by the env will take precedence over a matching entry in the config file or database, which can lead to some misleading behaviour when a value is set in multiple places.
So assuming we're doing something like:
```
On every scheduler start-up, check the environment for env vars like `AIRFLOW_POOL_<POOL_NAME>=<size>`
For every match, create or update the specified pool to the given size
```
A few uncertainties come to mind:
1) Are we OK with the pool being left there once the env var is removed?
2) Are we OK with the pool being reset to the env-var configured size after being manually updated via the CLI or UI? (since we'd be treating environment variables as the higher-priority source of truth)
Additionally, since there is a `description` field on Pools, we could update the description to say something like `Provisioned by AIRFLOW__POOL_MY_POOL environment variable` which might reduce some confusion around pool values changing without UI / CLI input.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] thcidale0808 commented on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
thcidale0808 commented on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-942716168
@fritz-astronomer are you happy with the scope/assumptions raised by @SamWheating here: https://github.com/apache/airflow/issues/18582#issuecomment-931473731 ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SamWheating commented on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
SamWheating commented on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-931473731
So we _can_ modify an env var, but the issue is that these changes aren't persisted anywhere and will be wiped or reset on a server or container restart, hence its not really a reliable place for long-lived read-write values.
So assuming we're doing something like:
```
On every scheduler start-up, check the environment for env vars like `AIRFLOW_POOL_<POOL_NAME>=<size>`
For every match, create or update the specified pool to the given size
```
A few uncertainties come to mind:
1) Are we OK with the pool being left there once the env var is removed?
2) Are we OK with the pool being reset to the env-var configured size after being manually updated via the CLI or UI? (since we'd be treating environment variables as the higher-priority source of truth)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-933026630
Yeah. That is an interesting one - I am not 100% sure whether this is a good idea to do it this way, to be honest.
Should it be done by scheduler ? Webservers ? Which one, if we have multiple of those? I think there is no good answer to that - similarly as in case of adding new users, I think it should be kept outside of existing airflow components. I feel we are trying to put some band-aid where we need a better-thought solution.
Similarly as we do not feel comfortable about running migrations automatically when we start airflow components, modifying the pools at startup of the "regular" components sounds like a bit too magical.
There are some issues with that which might be very counter-intuitive when we try to implement all the cases - including deletion/incremental updates.
However maybe there is a better solution to that.
Maybe that is the right moment that we think about a separate "airflow config-sync" (maybe better name could be invented) kind of component, wich could take care about configuring all DB-related configuration from a code:
* runing migrations
* creating users
* creating pools
* possibly updating Airlfow Variables/Connections from exported files
* maybe we have some other DB-related config that could come from outside too
I think that could nicely solve a number of problems we have (and which we tried to solve different for a number of different use cases - differently in the "user-community" chart, differently in the "official" chart, diferently in 'quick-start docker compose" and differently in the image.
Instead of running "commands" that we have now (airflow db upgrade, airflow users create etc.) it could use files to store desired configuration state and it could do everythig that is needed to bring the configuration to the state that is described in the files.
It would serve similar purpose as the `terraform "state"` description - this would make the approach of "infrastrucure as a code" much easier and more straightforward. We could run such "config-sync" and make airflow depend on it being "ready" or "complete" in order to run and replace the "post-install" step of ours in the Helm chart if we make proper dependency management between components - same in docker-compose.
I saw a number of questions from users who wanted to do it (and some of the features of the user-community chart implemented were responding to that - not in a best way as there was no good support from Airflow).
And if we make such "config-sync" a first-class citizen in Airflow, we might address the need of our users in elegant and manageable way.
Just a thought - I'd love to know what you think about it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] thcidale0808 commented on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
thcidale0808 commented on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-932532164
I'm happy to contribute with this one. Can this be assigned to me?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SamWheating commented on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
SamWheating commented on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-931459195
I like this idea, but I'm not sure how it would work as Pools are read _and updated_ by the Airflow scheduler, whereas configuration, connections and variables from environment variables are read-only.
Maybe we could have something on scheduler start-up which checks for `AIRFLOW__POOL_*` environment variables and creates / updates pool sizes accordingly?
I'd be happy to work on this, but I think I need to understand the requirements better first. Could you expand on which aspects of pool configuration you want to manage via environment variables?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] fritz-astronomer commented on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
fritz-astronomer commented on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-931461737
I'd imagine that'd be it - @SamWheating . Create and update upon startup, would be enough for most use-cases that I can imagine. I imagine it would need to be semantically the same as with Vars / Connections set via the ENV (read-only, unable to modify? I've never actually tried to modify a Var that was set via the ENV)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-933026630
Yeah. That is an interesting one - I am not 100% sure whether this is a good idea to do it this way, to be honest.
Should it be done by scheduler ? Webservers ? Which one, if we have multiple of those? I think there is no good answer to that - similarly as in case of adding new users, I think it should be kept outside of existing airflow components. I feel we are trying to put some band-aid where we need a better-thought solution.
Similarly as we do not feel comfortable about running migrations automatically when we start airflow components, modifying the pools at startup of the "regular" components sounds like a bit too magical.
There are some issues with that which might be very counter-intuitive when we try to implement all the cases - including deletion/incremental updates.
However maybe there is a better solution to that.
Maybe that is the right moment that we think about a separate "airflow config-sync" (maybe better name could be invented) kind of component, wich could take care about configuring all DB-related configuration from a code:
* runing migrations
* creating users
* creating pools
* possibly updating Airlfow Variables/Connections from exported files
* maybe we have some other DB-related config that could come from outside too
I think that could nicely solve a number of problems we have (and which we tried to solve different for a number of different use cases - differently in the "user-community" chart, differently in the "official" chart, diferently in 'quick-start docker compose" and differently in the image.
Instead of running "commands" that we have now (airflow db upgrade, airflow users create etc.) it could use files to store desired configuration state and it could do everythig that is needed to bring the configuration to the state that is described in the files.
It would serve similar purpose as the `terraform "state"` description - this would make the approach of "infrastrucure as a code" much easier and more straightforward. We could run such "config-sync" and make airflow depend on it being "ready" or "complete" in order to run and replace the "post-install" step of ours in the Helm chart if we make proper dependency management between components - same in docker-compose.
I saw a number of questions from users who wanted to do it (and some of the features of the user-community chart implemented were responding to that - not in a best way as there was no good support from Airflow).
And if we make such "config-sync" a first-class citizen in Airflow, we might address the need of our users in elegant and manageable way.
Just a thought - I'd love what you think about it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-933026630
Yeah. That is an interesting one - I am not 100% sure whether this is a good idea to do it this way, to be honest.
Should it be done by scheduler ? Webservers ? Which one, if we have multiple of those? I think there is no good answer to that - similarly as in case of adding new users, I think it should be kept outside of existing airflow components. I feel we are trying to put some band-aid where we need a better-thought solution.
Similarly as we do not feel comfortable about running migrations automatically when we start airflow components, modifying the pools at startup of the "regular" components sounds like a bit too magical.
There are some issues with that which might be very counter-intuitive when we try to implement all the cases - including deletion/incremental updates.
However maybe there is a better solution to that.
Maybe that is the right moment that we think about a separate "airflow config-sync" (maybe better name could be invented) kind of component, wich could take care about configuring all DB-related configuration from a code:
* runing migrations
* creating users
* creating pools
* possibly updating Airlfow Variables/Connections from exported files
* maybe we have some other DB-related config that could come from outside too
I think that could nicely solve a number of problems we have (and which we tried to solve different for a number of different use cases - differently in the "user-community" chart, differently in the "official" chart, diferently in 'quick-start docker compose" and differently in the image.
Instead of running "commands" that we have now (airflow db upgrade, airflow users create etc.) it could use files to store desired configuration state and it could do everythig that is needed to bring the configuration to the state that is describe in the files.
It would serve similar purpose as the `terraform "state"` description - this would make the approach of "infrastrucure as a code" much easier and more straightforward. We could run such "config-sync" and make airflow depend on it being "ready" or "complete" in order to run and replace the "post-install" step of ours in the Helm chart if we make proper dependency management between components - same in docker-compose.
I saw a number of questions from users who wanted to do it (and some of the features of the user-community chart implemented were responding to that - not in a best way as there was no good support from Airflow).
And if we make such "config-sync" a first-class citizen in Airflow, we might address the need of our users in elegant and manageable way.
Just a thought - I'd love what you think about it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SamWheating edited a comment on issue #18582: Allow Pools to be set via ENV
Posted by GitBox <gi...@apache.org>.
SamWheating edited a comment on issue #18582:
URL: https://github.com/apache/airflow/issues/18582#issuecomment-931473731
So we _can_ modify an env var, but the issue is that these changes aren't persisted anywhere and will be wiped or reset on a server or container restart, hence its not really a reliable place for long-lived read-write values.
So assuming we're doing something like:
```
On every scheduler start-up, check the environment for env vars like `AIRFLOW_POOL_<POOL_NAME>=<size>`
For every match, create or update the specified pool to the given size
```
A few uncertainties come to mind:
1) Are we OK with the pool being left there once the env var is removed?
2) Are we OK with the pool being reset to the env-var configured size after being manually updated via the CLI or UI? (since we'd be treating environment variables as the higher-priority source of truth)
Additionally, since there is a `description` field on Pools, we could update the description to say something like `Provisioned by AIRFLOW__POOL_MY_POOL environment variable` which might reduce some confusion around pool values changing without UI / CLI input.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org