Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-25 Thread via GitHub
jedcunningham closed pull request #38876: Fix trigger_kwargs encryption/decryption on db migration URL: https://github.com/apache/airflow/pull/38876 -- 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

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-25 Thread via GitHub
jedcunningham commented on PR #38876: URL: https://github.com/apache/airflow/pull/38876#issuecomment-2078016764 Closing this as #39246 has been merged. Thanks for kicking off fixing this bug @hussein-awala! -- This is an automated message from the Apache Git Service. To respond to the

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-24 Thread via GitHub
jedcunningham commented on PR #38876: URL: https://github.com/apache/airflow/pull/38876#issuecomment-2076404146 I've opened #39246 as an alternative solution to this. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-24 Thread via GitHub
jedcunningham commented on PR #38876: URL: https://github.com/apache/airflow/pull/38876#issuecomment-2076305345 Currently downgrade also doesn't work when you have triggers - alembic tries to switch the column back to json before decrypting happens. -- This is an automated message from

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-24 Thread via GitHub
dstandish commented on PR #38876: URL: https://github.com/apache/airflow/pull/38876#issuecomment-2076067502 > > Should we consider yanking 2.9.0 once 2.9.1 is out with this fix? > > I don't think so. In my point of view if we wanted to yank then we should have merge this quickly and

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-24 Thread via GitHub
jedcunningham commented on PR #38876: URL: https://github.com/apache/airflow/pull/38876#issuecomment-2076010902 Just confirmed my suspicions, it doesn't work for offline migrations: ``` [2024-04-24T23:08:17.337+] {triggerer_job_runner.py:341} ERROR - Exception when executing

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-24 Thread via GitHub
jedcunningham commented on PR #38876: URL: https://github.com/apache/airflow/pull/38876#issuecomment-2075998782 Ignoring that the conditional is wrong, I don't follow how this migration approach works for offline migration in the first place. Am I missing functionality somewhere that

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-24 Thread via GitHub
eladkal commented on PR #38876: URL: https://github.com/apache/airflow/pull/38876#issuecomment-2075624714 > Should we consider yanking 2.9.0 once 2.9.1 is out with this fix? I don't think so. In my point of view if we wanted to yank then we should have merge this quickly and cut

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-24 Thread via GitHub
dstandish commented on PR #38876: URL: https://github.com/apache/airflow/pull/38876#issuecomment-2075334734 Should we consider yanking 2.9.0 once 2.9.1 is out with this fix? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-24 Thread via GitHub
Taragolis commented on code in PR #38876: URL: https://github.com/apache/airflow/pull/38876#discussion_r1577540292 ## tests/utils/test_db.py: ## @@ -143,23 +143,29 @@ def test_offline_upgrade_wrong_order(self, from_revision, to_revision): ], ) def

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-24 Thread via GitHub
uranusjr commented on code in PR #38876: URL: https://github.com/apache/airflow/pull/38876#discussion_r1577341418 ## airflow/utils/db.py: ## @@ -1666,11 +1667,14 @@ def upgradedb( _reserialize_dags(session=session) add_default_pool_if_not_exists(session=session)

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-24 Thread via GitHub
uranusjr commented on code in PR #38876: URL: https://github.com/apache/airflow/pull/38876#discussion_r1577334918 ## airflow/utils/db.py: ## @@ -1666,11 +1667,14 @@ def upgradedb( _reserialize_dags(session=session) add_default_pool_if_not_exists(session=session)

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-23 Thread via GitHub
Lee-W commented on code in PR #38876: URL: https://github.com/apache/airflow/pull/38876#discussion_r1577081620 ## airflow/utils/db.py: ## @@ -1666,11 +1667,14 @@ def upgradedb( _reserialize_dags(session=session) add_default_pool_if_not_exists(session=session)

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-17 Thread via GitHub
hussein-awala commented on code in PR #38876: URL: https://github.com/apache/airflow/pull/38876#discussion_r1569650413 ## airflow/utils/db.py: ## @@ -1666,11 +1667,14 @@ def upgradedb( _reserialize_dags(session=session)

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-17 Thread via GitHub
hussein-awala commented on code in PR #38876: URL: https://github.com/apache/airflow/pull/38876#discussion_r1569650144 ## airflow/utils/db.py: ## @@ -1666,11 +1667,14 @@ def upgradedb( _reserialize_dags(session=session)

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-16 Thread via GitHub
ephraimbuddy commented on code in PR #38876: URL: https://github.com/apache/airflow/pull/38876#discussion_r1566875191 ## tests/utils/test_db.py: ## @@ -143,23 +143,29 @@ def test_offline_upgrade_wrong_order(self, from_revision, to_revision): ], ) def

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-16 Thread via GitHub
ephraimbuddy commented on code in PR #38876: URL: https://github.com/apache/airflow/pull/38876#discussion_r1566866008 ## airflow/utils/db.py: ## @@ -1666,11 +1667,14 @@ def upgradedb( _reserialize_dags(session=session)

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-16 Thread via GitHub
ephraimbuddy commented on code in PR #38876: URL: https://github.com/apache/airflow/pull/38876#discussion_r1566866008 ## airflow/utils/db.py: ## @@ -1666,11 +1667,14 @@ def upgradedb( _reserialize_dags(session=session)

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-16 Thread via GitHub
ephraimbuddy commented on code in PR #38876: URL: https://github.com/apache/airflow/pull/38876#discussion_r1566863812 ## airflow/utils/db.py: ## @@ -1666,11 +1667,14 @@ def upgradedb( _reserialize_dags(session=session)

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-15 Thread via GitHub
hussein-awala commented on PR #38876: URL: https://github.com/apache/airflow/pull/38876#issuecomment-2057810427 > Hi @hussein-awala , can you add more information to the commit message, I'm finding it difficult to understand what you worked on. I updated the PR description, could you

Re: [PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-14 Thread via GitHub
ephraimbuddy commented on PR #38876: URL: https://github.com/apache/airflow/pull/38876#issuecomment-2054195117 Hi @hussein-awala , can you add more information to the commit message, I'm finding it difficult to understand what you worked on. -- This is an automated message from the

[PR] Fix trigger_kwargs encryption/decryption on db migration [airflow]

2024-04-09 Thread via GitHub
hussein-awala opened a new pull request, #38876: URL: https://github.com/apache/airflow/pull/38876 closes: #38836 -- 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