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/11/23 18:31:01 UTC

[GitHub] [airflow] wesleyemery opened a new issue #19784: Airflow exception at login

wesleyemery opened a new issue #19784:
URL: https://github.com/apache/airflow/issues/19784


   ### Official Helm Chart version
   
   1.3.0 (latest released)
   
   ### Apache Airflow version
   
   2.1.0
   
   ### Kubernetes Version
   
   1.20.9
   
   ### Helm Chart configuration
   
   ```
   
   images:
     airflow:
       repository: "beertechcontainerregistrydev.azurecr.io/beertech-airflow-dev"
       tag: "dev-fb73985f1f2dbe7b7a2668c5a3fd0f541a5cf4cf"
     useDefaultImageForMigration: true
   
   nodeSelector:
     agentpool: "airflow"
   
   ingress:
     enabled: true
     web:
       annotations: 
         kubernetes.io/ingress.class: "nginx"
       hosts: ["airflow.dev.beertech.com"]
       tls:
         enabled: true
         secretName: ingress-tls-secret
     # flower:
     #   annotations: 
     #     kubernetes.io/ingress.class: "nginx"  
     #   path: "/flower"
     #   hosts: ["dev.airflow.beertech.com"]
     #   tls:
     #     enabled: true
     #     secretName: ingress-tls-secret
   webserver:
     defaultUser:
       enabled: true
       role: Admin
       username: devops
       email: nazbeertechdevops@AnheuserBuschInBev.onmicrosoft.com
       firstName: devops
       lastName: admin
       password: nottelling
   dags:
     gitSync:
       enabled: true
       repo: "ssh://git@github.com/ab-inbev-beertech/beertech-airflow.git"
       branch: "main"
       subPath: "dags/"
       sshKeySecret: airflow-ssh-secret
   extraSecrets:
     airflow-ssh-secret:
       data: |
         gitSshKey: ""nottellin"
   ```
   
   ### Docker Image customisations
   
   
   FROM apache/airflow:latest-python3.7
   
   USER root
   RUN sudo apt-get update \
     && sudo apt-get install -y g++ \
     && sudo apt-get install -y unixodbc-dev \
     && sudo apt-get install -y python3.7-dev \
     && python3.7 -m pip install --upgrade pip
   USER airflow
   COPY ./.netrc .
   RUN mv .netrc $HOME
   RUN pip install --no-cache-dir --user lola_utils --index-url https://abinbev.jfrog.io/artifactory/api/pypi/beerte-virtual-pypi/simple
   
   ### What happened
   
   I get this error at log in:
   ```
   Traceback (most recent call last):
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1277, in _execute_context
       cursor, statement, parameters, context
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/engine/default.py", line 593, in do_execute
       cursor.execute(statement, parameters)
   psycopg2.errors.UndefinedColumn: column dag.concurrency does not exist
   LINE 1: ..., dag.schedule_interval AS dag_schedule_interval, dag.concur...
                                                                ^
   
   
   The above exception was the direct cause of the following exception:
   
   Traceback (most recent call last):
     File "/home/airflow/.local/lib/python3.7/site-packages/flask/app.py", line 2447, in wsgi_app
       response = self.full_dispatch_request()
     File "/home/airflow/.local/lib/python3.7/site-packages/flask/app.py", line 1952, in full_dispatch_request
       rv = self.handle_user_exception(e)
     File "/home/airflow/.local/lib/python3.7/site-packages/flask/app.py", line 1821, in handle_user_exception
       reraise(exc_type, exc_value, tb)
     File "/home/airflow/.local/lib/python3.7/site-packages/flask/_compat.py", line 39, in reraise
       raise value
     File "/home/airflow/.local/lib/python3.7/site-packages/flask/app.py", line 1950, in full_dispatch_request
       rv = self.dispatch_request()
     File "/home/airflow/.local/lib/python3.7/site-packages/flask/app.py", line 1936, in dispatch_request
       return self.view_functions[rule.endpoint](**req.view_args)
     File "/home/airflow/.local/lib/python3.7/site-packages/airflow/www/auth.py", line 34, in decorated
       return func(*args, **kwargs)
     File "/home/airflow/.local/lib/python3.7/site-packages/airflow/www/views.py", line 547, in index
       filter_dag_ids = current_app.appbuilder.sm.get_accessible_dag_ids(g.user)
     File "/home/airflow/.local/lib/python3.7/site-packages/airflow/www/security.py", line 298, in get_accessible_dag_ids
       return {dag.dag_id for dag in accessible_dags}
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/orm/query.py", line 3535, in __iter__
       return self._execute_and_instances(context)
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/orm/query.py", line 3560, in _execute_and_instances
       result = conn.execute(querycontext.statement, self._params)
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1011, in execute
       return meth(self, multiparams, params)
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/sql/elements.py", line 298, in _execute_on_connection
       return connection._execute_clauseelement(self, multiparams, params)
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1130, in _execute_clauseelement
       distilled_params,
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1317, in _execute_context
       e, statement, parameters, cursor, context
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1511, in _handle_dbapi_exception
       sqlalchemy_exception, with_traceback=exc_info[2], from_=e
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/util/compat.py", line 182, in raise_
       raise exception
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1277, in _execute_context
       cursor, statement, parameters, context
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/engine/default.py", line 593, in do_execute
       cursor.execute(statement, parameters)
   sqlalchemy.exc.ProgrammingError: (psycopg2.errors.UndefinedColumn) column dag.concurrency does not exist
   LINE 1: ..., dag.schedule_interval AS dag_schedule_interval, dag.concur...
                                                                ^
   
   [SQL: SELECT dag.dag_id AS dag_dag_id, dag.root_dag_id AS dag_root_dag_id, dag.is_paused AS dag_is_paused, dag.is_subdag AS dag_is_subdag, dag.is_active AS dag_is_active, dag.last_parsed_time AS dag_last_parsed_time, dag.last_pickled AS dag_last_pickled, dag.last_expired AS dag_last_expired, dag.scheduler_lock AS dag_scheduler_lock, dag.pickle_id AS dag_pickle_id, dag.fileloc AS dag_fileloc, dag.owners AS dag_owners, dag.description AS dag_description, dag.default_view AS dag_default_view, dag.schedule_interval AS dag_schedule_interval, dag.concurrency AS dag_concurrency, dag.has_task_concurrency_limits AS dag_has_task_concurrency_limits, dag.next_dagrun AS dag_next_dagrun, dag.next_dagrun_create_after AS dag_next_dagrun_create_after 
   FROM dag]
   (Background on this error at: http://sqlalche.me/e/13/f405)
   ```
   
   ### What you expected to happen
   
   I expected to see the home page. 
   
   ### How to reproduce
   
   Here are the steps followed: 
   
   The purpose of this document is to outline the steps in deploying Apache Airflow which includes the infrastructure and the configuration settings to sync with your DAG workflows.
   
   The Airflow deployment includes the following:
   
   Azure Resource Group
   Kubernetes
   Add SSH Deploy Key to DAGS repository
   Add SSH key to Key Vault for Github authentication to DAGs repository
   Airflow Helm Deployment
   Nginx Ingress Controller Deployment
   DNS A record to the Ingress Load Balancer IP
   HTTPS enabled to the ingress
   All the modules used in the products repository can be found in the terraform-modules  GitHub repository.
   Be sure to use the most up to date modules
   
   Create Dev Infrastructure
   1. Create Resource group
   Clone terraform-products  and create a branch.
   Create product directory structure in the terraform-products  repository
   Create Dev resource group using this module 
   b. NOTE: Make sure all defaults are set for variables
   Create a pull request and merge to create the new Resource Group
   Verify Resource Group was created correctly in the Azure Portal 
   Example PR 
   
   2. Provide the non-prod service principal contributor and User Access Administrator access to the new resource group
   Go to Service Now 
   Click New IT Issue or Request"
   image.png
   Click "New Request"
   image.png
   Click "DC & Cloud - Manual Prov..."
   image.png
   Select Access Request From the "What do you want" drop-down menu
   Fill out the following information as seen below replacing Group with your new RG
   image.png
   3. Provide contributor access to Resource Group
   Create and checkout branch in the terraform-products  repository
   Modify the core-infra/main.tf to include the resource-group-ad module 
   Add NAZ BeerTech Admin Prod, <NEW AAD GROUP> as contributors
   Add NAZ BeerTech Admin Dev as a reader
   Commit changes and remain on a branch for the next step.
   4. Create Azure Kubernetes Service
   Create a new folder with the following relative path, <product dir>/dev/azure/kubernetes
   
   Use the AKS module  to create the required files.
   a. NOTE: Make sure all defaults are set for variables
   
   Commit changes.
   
   Create a pull request and merge after approvals.
   
   Add acrPull role to cluster scoped to the beertechcontainerregistrydev.azurecr.io  registry.
   a. Open a RITM in ServiceNow with the cloud engineering team to have an engineer with Owner permissions on the subscription to run the following command: az aks update --name <cluster name> --resource-group <resource group> --attach-acr beertechcontainerregistrydev
   
   Example:
   
   module "kubernetes" {
     source = "github.com/ab-inbev-beertech/terraform-modules.git//azurerm/aks?ref=main"
   
     location = "centralus"
     rg_name  = "BEERTECH-ORCHESTRATION-RG-NAZ-DEV"
   
     product_name         = "airflow"
     project_name         = "airflow"
     environment          = "dev"
     department           = "test"
     business_owner_email = "philip.theron@anhesuer-busch.com"
     dev_owner_email      = "pranavraj.tyagi@anhesuer-busch.com"
     criticality          = "low"
     cost_center          = "test"
     maintenance_window   = "7-9am"
   
     default_node_pool = {
       name                         = "system"
       availability_zones           = [1, 2, 3]
       enable_auto_scaling          = true
       min_count                    = 1
       max_count                    = 5
       node_count                   = null
       node_labels                  = {}
       only_critical_addons_enabled = false
       os_disk_size_gb              = 128
       os_disk_type                 = "Managed"
       type                         = "VirtualMachineScaleSets"
       vm_size                      = "Standard_B2s"
       subnet_id                    = null
     }
   
     node_pools = {
       airflow = {
         availability_zones           = [1, 2, 3]
         enable_auto_scaling          = true
         enable_host_encryption       = false
         enable_node_public_ip        = true
         min_count                    = 1
         max_count                    = 3
         node_count                   = null
         node_labels                  = {}
         only_critical_addons_enabled = false
         os_disk_size_gb              = 128
         os_disk_type                 = "Managed"
         type                         = "VirtualMachineScaleSets"
         vm_size                      = "Standard_B4ms"
         subnet_id                    = null
       }
     }
   5. Add SSH Deployment Key to DAGS Repository
   Create your SSH Key: ssh-keygen -t rsa -b 4096 -C "your_email@example.com"
   Add the public key to your private repo with the DAGs (under Settings > Deploy keys).
   Example DAGs Repo 
   6. Create Key Vault Secret for SSH Key
   Go to the Azure Portal 
   Access the Key Vault in which you will add your Base64 encoded SSH Private Key.
   a. To Base64 encode your private key rn base64 ./<path to private key>
   Select ( under Settings > Secrets > + Generate/Import)
   Screen Shot 2021-10-08 at 10.15.55 AM.png
   Add your Secret name and value and select create.
   Screen Shot 2021-10-08 at 10.16.55 AM.png
   7. Create TLS Kubernetes Secret with TLS Cert
   Run the command: kubectl create secret tls ingress-tls-secret -n airflow --key ./beertech-generated-private-key.key --cert cert.crt
   Note: The ingress of for Airflow will use this secret ingress-tls-secret needs to be added as an input for the secret_name variable
   
   8. Nginx Controller Deployment
   Create a nginx.tf file in the <product dir>/dev/azure/kubernetes folder.
   Add the necessary variable to the variables.tf file.
   a. NOTE: Make sure all defaults are set for variables
   Commit changes.
   Create a pull request and merge after approvals.
   Example:
   
   module "nginx" {
     source           = "github.com/ab-inbev-beertech/terraform-modules.git//helm/nginx-ingress?ref=main"
     depends_on       = [module.kubernetes]
     namespace        = var.nginx_namespace
     create_namespace = true
     ingress_type     = "Public"
   }
   9. DNS A record to the Ingress Load Balancer IP
   Add A record to existing zone in products, products/devops/prod/dns-zones/beertech-com
   
   Commit changes.
   
   Create a pull request and merge after approvals.
   Example:
   
    # A Records
     a_records = {
       #Beertech.com App Service Production. Domain: beertech.com
       beertech-prod-a = {
         domain_name  = "@"
         ip_addresses = ["104.43.221.31"]
       }
       beertech-airflow-dev = {
         domain_name  = "airflowdev"
         ip_addresses = ["20.112.200.142"]
       }
     }
   Note: The IP of the A record should match the LoadBalancer IP of ingress
   
   10. Create Airflow Helm AKS Deployment
   Create a airflow.tf file in the <product dir>/dev/azure/kubernetes folder.
   Add the necessary variable to the variables.tf file.
   a. NOTE: Make sure all defaults are set for variables
   Use Key Vault data resource to get the SSH key
   Example:
   data "azurerm_key_vault" "key-vault-name" {
     name                = "BeertechNonProdKeyVault"
     resource_group_name = "BEERTECH-DEVOPS-RG-NAZ-DEV"
   }
   
   data "azurerm_key_vault_secret" "ssh-key" {
     key_vault_id = data.azurerm_key_vault.key-vault-name.id
     name         = "AIRFLOW-SSH-DEPLOY-KEY"
   }
   
   data "azurerm_key_vault_secret" "airflow-admin" {
     key_vault_id = data.azurerm_key_vault.key-vault-name.id
     name         = "AIRFLOW-ADMIN"
   }
   Commit changes.
   Create a pull request and merge after approvals.
   Example Airflow deployment:
   
   module "airflow" {
     source              = "github.com/ab-inbev-beertech/terraform-modules.git//helm/airflow?ref=main"
     depends_on          = [module.kubernetes]
     namespace           = "airflow"
     create_namespace    = true
     airflow_repository  = "beertechcontainerregistrydev.azurecr.io/beertech-airflow-dev"
     airflow_tag         = "dev-fb73985f1f2dbe7b7a2668c5a3fd0f541a5cf4cf"
     persistence_enabled = false
     git_sync_enabled    = true
     dags_repository     = "ssh://git@github.com/ab-inbev-beertech/beertech-airflow.git"
     dags_branch         = "main"
     dags_subpath        = "dags/"
     ssh_key             = data.azurerm_key_vault_secret.ssh-key.value
     node_pool_selector  = "airflow"
     admin_username      = "devops"
     admin_password      = data.azurerm_key_vault_secret.airflow-admin.value
     admin_email         = "nazbeertechdevops@AnheuserBuschInBev.onmicrosoft.com"
     ingress_enabled     = true
     hostname            = "airflowdev.beertech.com"
     secret_name         = "ingress-tls-secret"
   }
   
   Note: The above code does the following:
   
   Deploys Airflow on the Kubernetes cluster.
   Pulls Airflow custom image from ACR.
   Uses SSH Key to Git Sync with the dags in the Github repository.
   Creates Ingress to Airflow
   Adds TLS Secret to ingress
   Updates default admin
   
   ### Anything else
   
   This happens every time at login 
   
   ### Are you willing to submit PR?
   
   - [X] 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] potiuk commented on issue #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-978143282


   Interesting thign is that you have missing `airflow` installation when you wanted to upgrade. That indicates that maybe you used wrong namespace or previously installed it differently. I think you need to debug that first.


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977310211


   > We already do that, the issue actually is we/user need to check logs for the specific container,
   
   This is good. The problem with it was that did not happen - simply because both "migrate" job and the `wait-for-migration` container completed successfullly - they "migrated" to version 2.1.0, But the "regular"  image was 2.2.0 and failed with missing column. So this is the main problem to handle here.
   
   So I think what can be done (as I proposed above) 
   
   1) change the "wait-for-migration" container to use the same image as the "regular" airflow components rathe than the "migration" one. Then I agree the error will show up as needed
   
   also I would simply add this to the message:
   
   2) "If you have `useDefaultImageForMigration` set to `true`, please make sure airflow version in this image is the same as in your main image"
   
   I simply anticipate this will happen quite often that people will not realise that they should also upgrade the "migration" image and having a helpful hint does not hurt, does 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] jedcunningham commented on issue #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
jedcunningham commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977034398


   I'm a little torn on this one. The comment seems pretty descriptive of what that flag does and calls out exactly what image will be used 🤷‍♂️:
   
   https://github.com/apache/airflow/blob/bccb45f5fe067ffa64f5f303bfbb6e8c1b552add/chart/values.yaml#L54-L58
   
   I think the bigger issue is folks reach for `images.airflow.repository` over `defaultAirflowRepository` because the former seems more intuitive. This is something on my personal wish-list to change when we start doing 2.0.0.
   
   All that said, I am a little surprised the wait-for-migrations init container didn't prevent the components from starting.


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977752962


   I can do it as well :). I think the more important change is whether it's OK/good to change the "wait-for-migration" image to be the same as "regular airflow" image? Not 100% sure what was the decision for having this defaultImage but until we change:        -
   ```
    name: wait-for-airflow-migrations
           resources:
             {{- toYaml .Values.triggerer.resources | nindent 12 }}
           image: {{ template "airflow_image_for_migrations" . }}
   ```
   
   into:
   
   ```
    name: wait-for-airflow-migrations
           resources:
             {{- toYaml .Values.triggerer.resources | nindent 12 }}
           image: {{ template "airflow_image" . }}
   ```
   
   The user (as in @wesleyemery  case) will not receive the error message at all in case the versions are different. 
   
   


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977752962


   I can do it as well :). I think the more important change is whether it's OK/good to change the "wait-for-migration" image to be the same as "regular airflow" image? Not 100% sure what was the decision for having this "arflow_image_for_migrations"  but until we change:  
   ```
    name: wait-for-airflow-migrations
           resources:
             {{- toYaml .Values.triggerer.resources | nindent 12 }}
           image: {{ template "airflow_image_for_migrations" . }}
   ```
   
   into:
   
   ```
    name: wait-for-airflow-migrations
           resources:
             {{- toYaml .Values.triggerer.resources | nindent 12 }}
           image: {{ template "airflow_image" . }}
   ```
   
   The user (as in @wesleyemery  case) will not receive the error message at all in case the versions are different. 
   
   


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977310211


   > We already do that, the issue actually is we/user need to check logs for the specific container,
   
   This is good. The problem with it was that did not happen - simply because both "migrate" job and the `wait-for-migration` container completed successfullly - they "migrated" to version 2.1.0, But the "regular"  image was 2.2.0 and failed with missing column. So this is the main problem to handle here.
   
   So I think what can be done (as I proposed above) 
   
   1) change the "wait-for-migration" container to use the same image as the "regular" airflow components rathe than the "migration" one. Then I agree the error will show up as needed
   
   also I would simply add 
   
   2) "If you are using `useDefaultImageForMigration`, make sure airflow version in this image is the same as in your main image"
   
   I simply anticipate this will happen quite often that people will not realise that they should also upgrade the "migration" image and having a helpful hint does not hurt, does 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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977042973


   > All that said, I am a little surprised the wait-for-migrations init container didn't prevent the components from starting.
   
   It did not, because it was using the very same 'defult airflow image" (which was different version than the "regular image" - this is the whole point :)


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977042973


   > All that said, I am a little surprised the wait-for-migrations init container didn't prevent the components from starting.
   
   It did not, because it was using the very same 'defult airflow image" (which was different version than the "regular image") - this is the whole point :)  - both "migration" and "wait-for-migration" use that image. 
   
   
   ```
         initContainers:
           - name: wait-for-airflow-migrations
             resources:
   {{ toYaml .Values.scheduler.resources | indent 12 }}
             image: {{ template "airflow_image_for_migrations" . }}
             imagePullPolicy: {{ .Values.images.airflow.pullPolicy }}
             args:
             {{- include "wait-for-migrations-command" . | indent 10 }}
             envFrom:
   ```


-- 
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] wesleyemery commented on issue #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
wesleyemery commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-978113168


   > he problem is that your "custom" image is likely based on 2.2.2 version (which is current latest) but since you have `useDefaultImageForMigration`, you are using `defaultAirflowTag` which is probably 2.1.0 as this was the default version released together with the chart. Which means that migration did not actually migrate the database as needed by A
   
   the base image was 2.1.0. I added the 2.2.2 version to the base image to match what helm is expected to see and I still get an error.
   ```
   Something bad has happened.
   Please consider letting us know by creating a bug report using GitHub.
   
   Python version: 3.7.12
   Airflow version: `2.0.2`
   Node: airflow-webserver-5f6d7b6c74-9d772
   -------------------------------------------------------------------------------
   Traceback (most recent call last):
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1277, in _execute_context
       cursor, statement, parameters, context
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/engine/default.py", line 593, in do_execute
       cursor.execute(statement, parameters)
   psycopg2.errors.UndefinedColumn: column dag.concurrency does not exist
   LINE 1: ..., dag.schedule_interval AS dag_schedule_interval, dag.concur...
                                                                ^
   
   
   The above exception was the direct cause of the following exception:
   
   Traceback (most recent call last):
     File "/home/airflow/.local/lib/python3.7/site-packages/flask/app.py", line 2447, in wsgi_app
       response = self.full_dispatch_request()
     File "/home/airflow/.local/lib/python3.7/site-packages/flask/app.py", line 1952, in full_dispatch_request
       rv = self.handle_user_exception(e)
     File "/home/airflow/.local/lib/python3.7/site-packages/flask/app.py", line 1821, in handle_user_exception
       reraise(exc_type, exc_value, tb)
     File "/home/airflow/.local/lib/python3.7/site-packages/flask/_compat.py", line 39, in reraise
       raise value
     File "/home/airflow/.local/lib/python3.7/site-packages/flask/app.py", line 1950, in full_dispatch_request
       rv = self.dispatch_request()
     File "/home/airflow/.local/lib/python3.7/site-packages/flask/app.py", line 1936, in dispatch_request
       return self.view_functions[rule.endpoint](**req.view_args)
     File "/home/airflow/.local/lib/python3.7/site-packages/airflow/www/auth.py", line 34, in decorated
       return func(*args, **kwargs)
     File "/home/airflow/.local/lib/python3.7/site-packages/airflow/www/views.py", line 497, in index
       filter_dag_ids = current_app.appbuilder.sm.get_accessible_dag_ids(g.user)
     File "/home/airflow/.local/lib/python3.7/site-packages/airflow/www/security.py", line 273, in get_accessible_dag_ids
       return {dag.dag_id for dag in accessible_dags}
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/orm/query.py", line 3535, in __iter__
       return self._execute_and_instances(context)
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/orm/query.py", line 3560, in _execute_and_instances
       result = conn.execute(querycontext.statement, self._params)
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1011, in execute
       return meth(self, multiparams, params)
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/sql/elements.py", line 298, in _execute_on_connection
       return connection._execute_clauseelement(self, multiparams, params)
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1130, in _execute_clauseelement
       distilled_params,
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1317, in _execute_context
       e, statement, parameters, cursor, context
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1511, in _handle_dbapi_exception
       sqlalchemy_exception, with_traceback=exc_info[2], from_=e
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/util/compat.py", line 182, in raise_
       raise exception
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1277, in _execute_context
       cursor, statement, parameters, context
     File "/home/airflow/.local/lib/python3.7/site-packages/sqlalchemy/engine/default.py", line 593, in do_execute
       cursor.execute(statement, parameters)
   sqlalchemy.exc.ProgrammingError: (psycopg2.errors.UndefinedColumn) column dag.concurrency does not exist
   LINE 1: ..., dag.schedule_interval AS dag_schedule_interval, dag.concur...
                                                                ^
   
   [SQL: SELECT dag.dag_id AS dag_dag_id, dag.root_dag_id AS dag_root_dag_id, dag.is_paused AS dag_is_paused, dag.is_subdag AS dag_is_subdag, dag.is_active AS dag_is_active, dag.last_parsed_time AS dag_last_parsed_time, dag.last_pickled AS dag_last_pickled, dag.last_expired AS dag_last_expired, dag.scheduler_lock AS dag_scheduler_lock, dag.pickle_id AS dag_pickle_id, dag.fileloc AS dag_fileloc, dag.owners AS dag_owners, dag.description AS dag_description, dag.default_view AS dag_default_view, dag.schedule_interval AS dag_schedule_interval, dag.concurrency AS dag_concurrency, dag.has_task_concurrency_limits AS dag_has_task_concurrency_limits, dag.next_dagrun AS dag_next_dagrun, dag.next_dagrun_create_after AS dag_next_dagrun_create_after 
   FROM dag]
   (Background on this error at: http://sqlalche.me/e/13/f405)
   ```
   
   When I set:
     useDefaultImageForMigration: false 
   
   I get a timeout waiting for a condition when installing the chart. I hope I'm not missing some detail due to user error. 
   
   ```
   FROM apache/airflow:2.2.2-python3.7
   
   USER root
   RUN sudo apt-get update \
     && sudo apt-get install -y g++ \
     && sudo apt-get install -y unixodbc-dev \
     && sudo apt-get install -y python3.7-dev \
     && python3.7 -m pip install --upgrade pip
   USER airflow
   COPY ./.netrc .
   RUN mv .netrc $HOME
   RUN pip install --no-cache-dir --user lola_utils --index-url https://abinbev.jfrog.io/artifactory/api/pypi/beerte-virtual-pypi/simple
   ```
   
   Here I explicitly added the version of airflow to match what's expected by default in `defaultAirflowTag`
   


-- 
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] github-actions[bot] commented on issue #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-1030717911


   This issue has been closed because it has not received response from the issue author.


-- 
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] boring-cyborg[bot] commented on issue #19784: Airflow exception at login

Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-976983460


   Thanks for opening your first issue here! Be sure to follow the issue template!
   


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977754602


   (Just wanted to make sure we all understand that this is the way to go :) 


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977047146


   My point here is that when you use 'useDefaultImageForMigration` you pretty much either get different versions for those two images when latest is used accidentally to rebuild the image (case above) or you have to always remember to update the version in two places (custom image and image for migrations). 
   
   This is completly non-intuitive and is pretty much guaranteed to happen at this or that migration. We shoudl have some protection to warn people when it happens.


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-978122119


   `images.airflow.pullPolicy`


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977004264


   The problem is that you did not change "defaultAirflowTag" but you rebuilt your image with "latest" version - which has been upgraded to 2.2.2. 
   
   If you want to keep 2.1.0 version of Airlfow you should explicitly base your image on it:
   In your docker image you shoud use:
   
   ```
   FROM apache/airflow:2.1.0-python3.7
   ```
   
   The `latest` tag is moving whenever we release new version.
   
   The problem is that your "custom" image is likely based on 2.2.2 version (which is current latest) but since you have `useDefaultImageForMigration`, you are using `defaultAirflowTag` which is probably 2.1.0 as this was the default version released together with the chart. Which means that migration did not actually migrate the database as needed by Airflow 2.2.2. 
   
   The solution is that you should make sure that both - your custom image and your `defaultAirflowTag` point to the same version. Likely what should work for you is to replace the `FROM` line with 2.1.0 (verify if your `defaultAirflowTag` is also 2.1.0, rebuild and push your image and make sure the new image is pulled when you deploy Airflow.
   
   


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977008352


   @jedcunningham @kaxil  - this is an interesting edge case. I think we shoudl somehow verify that the image that we run for migration is the same version as the "regular airflow image" - otherwise it might lead to this kind of problem. It seems ti is very easy to fall in this trap by the users. Not sure how we can do it (maybe we should have extra init containers in the migration step with `version` command and compare both outputs - if that is possible of course.


-- 
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] kaxil commented on issue #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977067781


   It is difficult to do that with Helm as you can't really verify directly using just the "tags" via `NOTES.txt`. Users can have Custom images where the tags might be different.
   
   Few things and suggestions:
   - I would never recommend using a "moving target" for Production deployments -- i.e. never use the "latest" docker image. You should always use an immutable image to avoid surprises. Airflow does not have a good downgrade strategy. Using "latest" means you are prone to un-intentional Upgrades including Major versions.
   - You should only override the default on `useDefaultImageForMigration: True` if you want to control the DB migrations to avoid accidental upgrades, which worked in your case as you were able to identify. If you would have kept it the default you would have unintentionally upgraded to 2.2.2


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977310211


   > We already do that, the issue actually is we/user need to check logs for the specific container,
   
   This is good. The problem with it was that did not happen - simply because both "migrate" job and the `wait-for-migration` container completed successfullly - they "migrated" to version 2.1.0, But the "regular"  image was 2.2.0 and failed with missing column. So this is the main problem to handle here.
   
   So I think what can be done (as I proposed above) 
   
   1) change the "wait-for-migration" container to use the same image as the "regular" airflow components rathe than the "migration" one. Then I agree the error will show up as needed
   
   also I would simply add this to the message:
   
   2) "If you have `useDefaultImageForMigration` set to `true`, please make sure airflow version in this image is the same as in your main image" (or similar)
   
   I simply anticipate this will happen quite often that people will not realise that they should also upgrade the "migration" image and having a helpful hint does not hurt, does 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] kaxil commented on issue #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977819123


   The idea is to have the same image in `wait-for-airflow-migrations` and `run-migrations` as they work hand in hand.  `wait-for-airflow-migrations` should just wait for migrations and not check for errors. This is done mainly because Helm does not have a concept of "sequencing of events/pods".
   
   The `run-migrations` runs the DB migrations, so if the DB changes have been applied it is already too late as the upgrades have happened and we don't have a good rollback story. 
   
   So I feel we should really document the implications of using `useDefaultImageForMigration`. Overriding the default should be a deliberate action. It is the same thing as updating the `airflowVersion` during Airflow Upgrades.


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-978121456


   Do you have `imagePullPolicy: always` set ? If not then you have to make sure that whenever you build your custom image, you tag it with unique tag and use that tag in your helm chart. 


-- 
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] github-actions[bot] commented on issue #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-1025011703


   This issue has been automatically marked as stale because it has been open for 30 days with no response from the author. It will be closed in next 7 days if no further activity occurs from the issue author.


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977008352


   @jedcunningham @kaxil  - this is an interesting edge case. I think we should somehow verify that the image that we run for migration is the same version as the "regular airflow image" - otherwise it might lead to this kind of problem. It seems ti is very easy to fall in this trap by the users. Not sure how we can do it (maybe we should have extra init containers in the migration step with `version` command and compare both outputs - if that is possible of course.


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977074265


   I still think documentation here is not enough, even if people will not use `latest` (which I agree is a very bad idea) they will always have to remember about upgrading two versions. This will fire back (users and us responding to issues) sooner or later IMHO. 
   
   Maybe the way to go is to actually change `wait-for-migration` to use the regular Image rather than the "migration image" ?  Sounds that `wait-for-migration` is far more safer to run using custom image?
   
   Even then the user scenario if somone does make the mistake is pretty bad . The nasty side effect that it will just be hanging forever without an "actionable" error or message (It will just wait for migrations to complete, but the migration will be completed.  However if we have reasonable timout for "wait-for-migration" and the error will include "Please check if your migration uses the  same version of airflow as the regular image" or something like that - that should be more than enough.


-- 
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] kaxil edited a comment on issue #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977067781


   It is difficult to do that with Helm as you can't really verify directly using just the "tags" via `NOTES.txt`. Users can have Custom images where the tags might be different.
   
   Few things and suggestions:
   - I would never recommend using a "moving target" for Production deployments -- i.e. never use the "latest" docker image. You should always use an immutable image to avoid surprises. Airflow does not have a good downgrade strategy. Using "latest" means you are prone to un-intentional Upgrades including Major versions.
   - You should only override the default on `useDefaultImageForMigration: True` if you want to control the DB migrations to avoid accidental upgrades, which works in probably all cases. We use it at Astronomer too and it has prevented tons of unintentional upgrades.


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977841918


   > So I feel we should really document the implications of using useDefaultImageForMigration. Overriding the default should be a deliberate action. It is the same thing as updating the airflowVersion during Airflow Upgrades.
   
   What I really want to achieve is so if somene makes such mistake, they get a clear information rather than "column is missing" (because it says nothing.
   
   Maybe what we really want is to prioritise merging of https://github.com/apache/airflow/pull/18439 in this case ? Because this will actuall work this way - airflow will refuse to start when the db will not be migrated ? Maybe we can merge it in 2.2.3 as well - to make it work soon ?
   
   I think it waits for your review @kaxil BTW :)
   


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-978142628


   You need to take a look at  what your error logs show from the containers that waited for installation to complete - inspect the pods that failed and see why - usually answer will be there. There is no way to help with the information we see now.


-- 
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] kaxil commented on issue #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977899027


   Yup agree on getting #18439 in but it is a new feature and has to be in 2.3.0 -- Want to really make sure (at least the goal is) there are no chances of introducing new bugs as far as possible in bug fixes.


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977047146


   My point here is that when you use 'useDefaultImageForMigration` you pretty much either get different versions for those two images when latest is used accidentally to rebuild the image (case above) or you have to always remember to update the version in two places (custom image and image for migrations). 
   
   This is completly non-intuitive and is pretty much guaranteed to happen at this or that migration. We should have some protection to warn people when it happens.


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977310211


   > We already do that, the issue actually is we/user need to check logs for the specific container,
   
   This is good. The problem with it was that did not happen - simply because both "migrate" job and the `wait-for-migration` container completed successfullly - they "migrated" to version 2.1.0, But the "regular"  image was 2.2.0 and failed with missing column. So this is the main problem to handle here.
   
   So I think what can be done (as I proposed above) 
   
   1) change the "wait-for-migration" container to use the same image as the "regular" airflow components rathe than the "migration" one. Then I agree the error will show up as needed
   
   also I would simply add this to the message:
   
   2) "If you are using `useDefaultImageForMigration`, make sure airflow version in this image is the same as in your main image"
   
   I simply anticipate this will happen quite often that people will not realise that they should also upgrade the "migration" image and having a helpful hint does not hurt, does 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] kaxil commented on issue #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977743106


   Yup, agreed on making it explicit in the messaging. We could also add a line or two in the docs on when you should use `useDefaultImageForMigration` and the steps to take care of, I will do it later today.


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977074265


   I still think documentation here is not enough, even if people will not use `latest` (which I agree is a very bad idea) they will always have to remember about upgrading two version. This will fire back (users and us responding to issues) sooner or later IMHO. 
   
   Maybe the way to go is to actually change `wait-for-migration` to use the regular Image rather than the "migration image" ?  Sounds that `wait-for-migration` is far more safer to run using custom image?
   
   Even then the user scenario if somone does make the mistake is pretty bad . The nasty side effect that it will just be hanging forever without an "actionable" error or message (It will just wait for migrations to complete, but the migration will be completed.  However if we have reasonable timout for "wait-for-migration" and the error will include "Please check if your migration uses the  same version of airflow as the regular image" or something like that - that should be more than enough.


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977752962


   I can do it as well :). I think the more important change is whether it's OK/good to change the "wait-for-migration" image to be the same as "regular airflow" image? Not 100% sure what was the decision for having this "arflow_image_for_migrations"  but until we change:  
   ```
    name: wait-for-airflow-migrations
           resources:
             {{- toYaml .Values.triggerer.resources | nindent 12 }}
           image: {{ template "airflow_image_for_migrations" . }}
   ```
   
   into:
   
   ```
    name: wait-for-airflow-migrations
           resources:
             {{- toYaml .Values.triggerer.resources | nindent 12 }}
           image: {{ template "airflow_image" . }}
   ```
   
   the user (as in @wesleyemery  case) will not receive the error message at all in case the versions are different. 
   
   


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977841918


   > So I feel we should really document the implications of using useDefaultImageForMigration. Overriding the default should be a deliberate action. It is the same thing as updating the airflowVersion during Airflow Upgrades.
   
   What I really want to achieve is so if somene makes such mistake, they get a clear information rather than "column is missing" (because it says nothing.
   
   Maybe what we really want is to prioritise merging of https://github.com/apache/airflow/pull/18439 in this case ? Because this will actuall work this way - airflow will refuse to start when the db will not be migrated ? Maybe we can merge it in 2.2.3 as well - to make it work soon ?
   
   
   
   


-- 
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] wesleyemery commented on issue #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
wesleyemery commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-978137403


   > not then you have to make sure that whenever you build your custom image, you tag it with unique tag and use that tag in your helm chart.
   
   Here is what I have: 
   ```
   airflowVersion: "2.2.2"
   defaultAirflowTag: "2.2.2"
   
   images:
     airflow:
       repository: "beertechcontainerregistrydev.azurecr.io/beertech-airflow-dev"
       tag: "dev-33c318f74394a79b30e65095bd7413541a9574c5"
       imagePullPolicy: always
   ```
   
   helm upgrade --install airflow apache-airflow/airflow -f ./airflow-override-values.yaml -n airflow 
   Release "airflow" does not exist. Installing it now.
   Error: failed post-install: timed out waiting for the condition


-- 
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] kaxil commented on issue #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977104481


   >I still think documentation here is not enough, even if people will not use latest (which I agree is a very bad idea) they will always have to remember about upgrading two versions. This will fire back (users and us responding to issues) sooner or later IMHO.
   
   Happy for you to suggest a solution that works with Helm and it's limitation :) We also have sensible defaults, so if someone changes the default, they really should make sure on the implications and we can document those implications, the actions are already clear in the docs on what to do when you change the default.
   
   >Even then the user scenario if somone does make the mistake is pretty bad . The nasty side effect that it will just be hanging forever without an "actionable" error or message (It will just wait for migrations to complete, but the migration will be completed). However if we have reasonable timeout for "wait-for-migration" and the error will include "Please check if your migration uses the same version of airflow as the regular image" or something like that - that should be more than enough.
   
   We already do that, the issue actually is we/user need to check logs for the specific container, 
   https://github.com/apache/airflow/blob/4be04143a5f7e246127e942bf1d73abcd22ce189/airflow/utils/db.py#L647-L651
   
   
   Example:
   
   ```
   ❯ k logs example-release-scheduler-7864b6ddd8-548rj -c wait-for-airflow-migrations
   ```
   
   To get the output.
   
   


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-978120465


   The log says your image is `2.0.2` so something is wrong with your images:
   
   ```
   Python version: 3.7.12
   Airflow version: `2.0.2`
   ```


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977074265


   I still think documentation here is not enough, even if people will not use `latest` (which I agree is a very bad idea) they will always have to remember about upgrading two versions. This will fire back (users and us responding to issues) sooner or later IMHO. 
   
   Maybe the way to go is to actually change `wait-for-migration` to use the regular Image rather than the "migration image" ?  Sounds that `wait-for-migration` is far more safer to run using custom image?
   
   Even then the user scenario if somone does make the mistake is pretty bad . The nasty side effect that it will just be hanging forever without an "actionable" error or message (It will just wait for migrations to complete, but the migration will be completed).  However if we have reasonable timeout for "wait-for-migration" and the error will include "Please check if your migration uses the  same version of airflow as the regular image" or something like that - that should be more than enough.


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977845057


   I think adding a message there also to make sure that the migration was run using the same version of airflow - might be handy


-- 
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 #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19784:
URL: https://github.com/apache/airflow/issues/19784#issuecomment-977042973


   > All that said, I am a little surprised the wait-for-migrations init container didn't prevent the components from starting.
   
   It did not, because it was using the very same 'defult airflow image" (which was different version than the "regular image") - this is the whole point :)  - both "migration" and "wait-for-migration" use that image. 


-- 
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] github-actions[bot] closed issue #19784: Airflow python exception at login

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed issue #19784:
URL: https://github.com/apache/airflow/issues/19784


   


-- 
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