jason810496 commented on issue #43897: URL: https://github.com/apache/airflow/issues/43897#issuecomment-2490939445
While working on this issue, I came up with two approaches and would appreciate some feedback to decide which one to pursue 🙏 ## Context for `Variable.set` For the `post_variable` implementation, the `Variable.set` method is used to add new variables. Ref: [variables.py#L155](https://github.com/apache/airflow/blob/main/airflow/api_fastapi/core_api/routes/public/variables.py#L155) The `Variable.set` method involves four steps: 1. **Check if the secret exists in the custom secrets backend** For example, if using Vault, AWS, GCP Secret Manager, etc., it will call `SecretsBackend.get_variable` to verify whether the variable is set in the external secret manager. 2. **Delete the old variable in the metastore** 3. **Add the variable with the new value** 4. **Set the secret in the cache** ## Approaches ### **1. Add a `get_variables` method to the `Variables` model and `SecretsBackend`** This approach would require implementing a `get_variables` method for both core and external secret managers: - **Core secret managers**: - Cache - Environment variables - Local filesystem - Metastore - **External secret managers**: - AWS - GCP - Vault - Azure - Yandex ### **2. Call `Variable.set` concurrently using `asyncio`** Instead of creating a `get_variables` method, this approach would involve calling `Variable.set` concurrently, and have to make sure there are no thread-safety issues. ## Pros and Cons ### **First Approach** - **Pros**: - Fewer API requests to external services. - **Cons**: - More work is required to implement the `get_variables` method across all secret backends. ### **Second Approach** - **Pros**: - Easier and faster to implement. - **Cons**: - May result in more API requests to external services, which could impact performance. - For example, setting up a bunch of variables in first time might call hundreds of API to external providers simultaneously. ## My opinion We can start with the **second approach** to quickly implement the main requirement (inserting multiple variables in FastAPI). Afterward, we can gradually refactor to use the **first approach** for better efficiency. cc @bugraoz93 @pierrejeambrun -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
