Ability to sync airflow variables during deployments #31875

Activity

1. javatarz

   added

   kind:featureFeature RequestsFeature Requests

   needs-triagelabel for new issues that we didn't triage yetlabel for new issues that we didn't triage yet

   on Jun 13, 2023
2. boring-cyborg commented on Jun 13, 2023

   

   boring-cyborgBot

   on Jun 13, 2023

   More actions

   Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.
3. javatarz commented on Jun 13, 2023

   

   javatarz

   on Jun 13, 2023

   ContributorAuthor

   More actions

   I'm happy to build this feature out in Airflow. I am about to do this as a script in our repository but thought that it might be more useful to contribute it back to Airflow. If the team thinks it's going to be a welcome addition to the Airflow CLI, I'll submit a PR with tests.
4. Adaverse commented on Jun 13, 2023

   

   Adaverse

   on Jun 13, 2023

   Contributor

   More actions

   I'm happy to build this feature out in Airflow. I am about to do this as a script in our repository but thought that it might be more useful to contribute it back to Airflow. If the team thinks it's going to be a welcome addition to the Airflow CLI, I'll submit a PR with tests.

   Please it would be a great addition!

   👍React with 👍1Reacted by Karun Japhet
5. raphaelauv commented on Jun 13, 2023

   

   raphaelauv

   on Jun 13, 2023

   Last edited by raphaelauv

   Contributor

   More actions

   create a concatenation scripts

   that create a unique .json file based on of all your .json files ( containing all your airflow variables )

   and then run

   airflow variables import all_variables.json

   same logic for pools and connections
6. potiuk commented on Jun 13, 2023

   

   potiuk

   on Jun 13, 2023

   Last edited by potiuk

   Member

   More actions

   We've been discussing those ideas few times and the result is pretty much always going the same route.

   This is quite a bigger task than you think it is and a number of things have to be discussed/proposed in order to be "ready" to accept it.

   I am not against the concept, I just want to be sure that whoever is going to propose a solution, is also going to see through all the consequences and proposes a solution that addresses the few issues issues I mention below.

   I propose that the author leads and gather feedback from others on the way how to solve that problem - and I think those who look for having this PR implemented should work out a complete and consistent proposal together. I have no strong opinions on that, I just know that this part should be addressed, so will keep listening and when the solution is agreed. Moreover we need to have documentation and be able to explain to our users what is the workflow they shoudl do and expect and have syncing implemented as feature in our Helm Chart. I think without it will be toothless - and it will raise a lot of support questions by people wanting to use it in our chart, so in order to accept that, we need to have answers. Also it's not entirely clear why LocalFilesystemBackend of ours is not fitting the use case needed here.

   More context:

   Editability of the variable/connection

   I would welcome this one if enabling it solves the problem of what happens when the user edits the connections of variables over UI or API only to find out that they were deleted or modify at random deployment times. This is the kind of surprises that our users do not want. If any kind of syncing is proposed, it also must address this part and it has to be consistent solution thinking about all the cases and scenarios.

   There are questions regarding which/whether and in which circumstances, the variables should be allowed to be changed in the database. What happens if you sync connections and want to add a new one and keep it persistent for example? How do define which of the connections are managed by the sync and which are managed outside? Or maybe the connections should not be editable at all? How to protect against such changes. How the users should see the connections /variables when they access the UI? should they see them at all? How to communiicate what is changeable and what is not. This means also probably proposing some UI changes and API changes that accomodate that. Also some thinking how the "airflow connection" and "aiflow variable" CLI commands should behave. Connected to that - how to control whether the connections can by synced/edited? Should it be controlled by configuration?

   Chart integration

   Another set of questions there regards the synchronisation scenarios in the chart of our. When and how such synchronisation should happen ? At the deployment time? When the file gets updated? Should it be via secret modification? Or via some other ways? When you re-synchronise (and potentially update the new values? This feature will only be really useful if we have a workflow defined in our chart that and instructions explaining how to do the synchronisation by the users. What the users should do to edit their connections? Should they redeploy chart? Or run some kind of migration? Or upgrade Airflow?

   LocalFileSystemBackend as an alternative

   Also we already have another solution for this kind of workflow when you want to make sure that whatever you configure in a file is visible as variable/connection namely LocalFilesystemBackend. It will take variables connections/variables from a file without syncing and make it available for workers and dag file processor, without introducing extra "sync-to-db" step. Might be not perfect (requires file syncing between all the components). and maybe there are other reasons why it's not good.

   https://airflow.apache.org/docs/apache-airflow/stable/administration-and-deployment/security/secrets/secrets-backend/local-filesystem-secrets-backend.html

   ---

   I think - somewhat surprisingly - the scope of this change is at the border at requiring an Airflow Improvement Proposal https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals . At the very least, this proposal should end up as a consistent proposal explaining all the details above in a devlist thread and at least calling for a [LAZY CONSENSUS] (https://www.apache.org/foundation/voting.html) because it has some unobvious consequences.

   Again - I have no strong opinions on those, I hope the discussion - here or on the devlist can propose a consistent solution for those problems (ideall worked out by those who are eager to have it) and then I am happy to accept it as is (as long as it is consistent and addresses those issues above).

   @javatarz - are you up to the challenge? and others - would you be happy in helping @javatarz to make a consistent proposal?

   ❤️React with ❤️4Reacted by Jens Scheffler, Akash Sharma, Karun Japhet and Mírian Batista
7. potiuk

   removed

   needs-triagelabel for new issues that we didn't triage yetlabel for new issues that we didn't triage yet

   on Jun 13, 2023
8. javatarz commented on Jun 14, 2023

   

   javatarz

   on Jun 14, 2023

   ContributorAuthor

   More actions

   Thank you for the detailed response @potiuk! There's a bunch of perspectives I hadn't thought of so I'll go through as much as I can answer now and for some of it, I'll have to come back.

   I would welcome this one if enabling it solves the problem of what happens when the user edits the connections of variables over UI or API only to find out that they were deleted or modify at random deployment times. This is the kind of surprises that our users do not want. If any kind of syncing is proposed, it also must address this part and it has to be consistent solution thinking about all the cases and scenarios.

   Our team's perspective on this has been that any airflow variable changes made through the UI will be temporary and will be over written in the next deployment. If we don't want this to happen, the change should have come from source control and not the UI. Ideally, we'd love to make the variables UI read only if that was a configuration (this isn't an option so far I believe).

   My proposal here would be to allow users to have an airflow config that allows the edit options in the variables section to be disabled. If people keep it enabled and use airflow variables sync in their deployments, we are OK with these variables being reset. While that creates confusion for some users, others might find it useful to make quick feature toggle/configuration changes in Airflow variables and are OK with them being reset in the next deployment.

   Changes proposed in this section:

   1. CLI command for sync: airflow variables sync file.json
   2. Configuration to disable UI edits for variables: AIRFLOW__UI__DISABLE_VARIABLE_EDITS

   What I dislike about this part of my proposal is that the default implementation is to allow variables sync and an editable UI and this might be confusing to some users. Changing the default behaviour will cause significantly more challenges so this is the best suggestion I can think of so far. What do others think?

   What happens if you sync connections and want to add a new one and keep it persistent for example?
   I'll try explaining how the sync works through an example

   Airflow state before sync:

   1. UnchangedVariable: UnchangedValue
   2. SameKeyDifferentValue: OldValue
   3. DeleteThisKey: DeleteThisValue

   Config file:

   {
     "UnchangedVariable": "UnchangedValue",
     "SameKeyDifferentValue": "NewValue",
     "NewKey": "BrandNewValue"
   }
   

   Airflow state after sync:

   1. UnchangedVariable: UnchangedValue
   2. SameKeyDifferentValue: NewValue
   3. NewKey: BrandNewValue

   Summary of changes:

   1. Keys and values that remain the same in Airflow vs the config will not be touched
   2. When values change, they will be updated
   3. When keys are removed, they will be deleted from airflow
   4. When keys are added, they will be added to airflow

   Another set of questions there regards the synchronisation scenarios in the chart of our.

   We do not use airflow's helm charts and will have to read up on these. I'd like to get back to this section of questions a bit later.

   What happens if you sync connections and want to add a new one and keep it persistent for example?

   Aren't connections non-editable at this point? We've been deleting and adding them for a while now :) I was hoping to start a separate card to talk about what we should do with connections. I'd love a similar sync capability but I'm not sure of the consequences of this. I'm not sure of the reasons why connections aren't editable. Is there somewhere I can read up on that discussion?

   Also we already have another solution for this kind of workflow when you want to make sure that whatever you configure in a file is visible as variable/connection namely LocalFilesystemBackend.

   Hoping to stay away from local file system changes post the deployment. Currently, I should be able to run this command if I have access to the terminal on a machine (SSH or the likes) and I'd like to avoid a common file system to keep all components in sync.

   ---

   I'm still up for the challenge. I'll start a discussion on the dev mailing list to talk about this change. Let's get consensus on what a consistent design should be. In the meanwhile, let me read up on parts that I don't know as much about yet (HelmCharts)
9. javatarz commented on Jun 14, 2023

   

   javatarz

   on Jun 14, 2023

   ContributorAuthor

   More actions

   create a concatenation scripts

   This is precisely what I meant in my original post when I said "The individual building blocks for this change are already available. The (design/philosophical) choice is between making Airflow easier to use and keeping airflow minimalistic i.e. provide the building blocks for users to build on". The blocks are available and it's fairly trivial to write a script to do this. Doing so would mean it becomes a small script that many Airflow consumers have to write. It's simple enough to build but would be better off as a capability in Airflow (despite it being a deceptively large change to make sure Airflow is consistent in it's interfaces).

   I'm happy writing up a script and pushing it on the thread if someone's in need of one. It'll be a script that you will have to maintain over a period of time. Our team already has a couple of such things. Every airflow upgrade feels more painful than it should be because we spend half our time just getting these utilities to work fine. One such example is what we've had to do for unit tests (which I'm hoping, some day, will become part of airflow's core offering too). But that's a discussion for another ticket on another day :) I seem to have bit off an AIP level change that I'd like to see through so baby steps.. :)
10. potiuk commented on Jun 14, 2023

    

    potiuk

    on Jun 14, 2023

    Last edited by potiuk

    Member

    More actions

    One other comment. Just to put it in persepective as well.

    Such script is super-easy to write. and it's maybe 10% of the work done to make such feature 'product' level. Bulk of work in the task will be exactly agreee and have a proposal to a complete solution that we can consider product feature in Airflow. Pushing a script which implements that, without having a "product" around it (i.e. documentation, workflows, instructions for users to use, considering edge cases and behaviours how this will work for existing users, configurability etc.) does more harm than good, because the first thing which will happen is our users (rightfully) will ask us "How should we use it? What happens if I do that?".

    My usual "rule of thumb" for those kind of changes is that it is roughly 10x more costly to make a product than your own dirty solution for such cases.

    Roughly:

    • (1) Writing a script that does it for you = 1x effort
    • (2) Pushing it out so it gets into somewhat reusable state for others with unit tests, regression prevention etc. = 3x (1)
    • (3) Making a product out of that = 3x (2) = 9x (1).

    This is mostly because of all the discussions, getting agreements and consensus of others, working out and documenting configuration options, defining and documenting workflows, Thinking through the edge cases,

    And in Airflow, we have to think in terms of product, I am afraid.
11. potiuk commented on Jun 14, 2023

    

    potiuk

    on Jun 14, 2023

    Member

    More actions

    We do not use airflow's helm charts and will have to read up on these. I'd like to get back to this section of questions a bit later.

    BTW. @javatarz -> you do not have to do it yourself. There are other people here who cheer that thread and want to get the feature in place, and some of them maybe have experience, use chart and have ideas how to integrate it there. So maybe they can help to hash out all the answers there (and BTW. Finding, encouraging, enthusing, and coordinating discussion and work around such features is big part of the 9x)