github-actions[bot] closed pull request #37638: Migrate to connexion v3
URL: https://github.com/apache/airflow/pull/37638
--
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 uns
github-actions[bot] commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2266291533
This pull request has been automatically marked as stale because it has not
had recent activity. It will be closed in 5 days if no further activity occurs.
Thank you for you
potiuk commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2058561523
Continued in #39055
--
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.
potiuk commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2058501665
> Hi @potiuk. Great to hear that you found the issue. Let me know if there
is anything else I can help with.
Hey @RobbeSneyders Thanks for the offer. We got the PR green finally
RobbeSneyders commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2057667740
Hi @potiuk. Great to hear that you found the issue. Let me know if there is
anything else I can help with.
--
This is an automated message from the Apache Git Service.
To respon
potiuk commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2056937741
CC: @RobbeSneyders : I think @Taragolis found the reason and I am going to
fix it - the error was misleading but came from a dangling `airflow` sources in
the image, where swagger was su
potiuk commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2056644934
Reproducible easily with"
* docker pull
ghcr.io/apache/airflow/main/prod/python3.8:ae320ca16ec3828656d57ff00e2cde50c74d7d1e
(pulls the prod-image built in this PR)
* docker ru
potiuk commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2056627808
Hello @RobbeSneyders : I think we need your expert help here.
We are quite far with making all our tests work with the new Connexion 3
(basically - they all work and we found and f
Satoshi-Sh commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2041152227
I'm working on `tests/api_connexion/endpoints/test_dataset_endpoint.py`.
 ?
--
This is an automa
Satoshi-Sh commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2019084388
I just fixed the test_cors.py. There might be an improvement in refactoring.
If someone can review it, it would be great.
https://github.com/sudiptob2/airflow/pull/35
Satoshi-Sh commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2016090236
I've been working on `tests/api_connexion/test_cors.py`. It would be great
if you could give me some inputs on this.
You can find the details
[here](https://github.com/sudipto
potiuk commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2015695912
> ### Does not raise Valueerror
When I run it in my unit test, I got Internal Server error - that's why it
does not return ValueError, because it converts ValueError into 500 inter
sudiptob2 commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2015632855
> Since is it API experimental (which is deprecated), I would not spend too
much time on it. Maybe change the expected exception? Change it to expect
`ValueError` from `connexion` lib
vincbeck commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2015557108
> ### Does not raise Valueerror
> This test seems not to raise `ValueError` anymore after migrating to
connexion v3. See **screenshot 1**. But a ValueError is indeed raised from deep
sudiptob2 commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2015521624
### Does not raise Valueerror
This test seems not to raise `ValueError` anymore after migrating to
connexion v3. See **screenshot 1**. But a ValueError is indeed raised from
deep
vincbeck commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2012298634
> Satoshi-Sh
I commented your PR
--
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
Satoshi-Sh commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2011109441
I had the same issue with assertion code status 302 == 200. It seems the
responses are after a user gets redirected to the targeted pages. I used the
path to check if the redirect w
sudiptob2 commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2010952631
Hi, @vincbeck
Need a little bit of help here.
As we are using `connexion_app.client` now, `anonymous_client` does not
return `302`when we call `pool/delete/id` route.
As a r
vincbeck commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2010475846
> After Jarek fixed the CI image, I'm able to see the same errors locally.
>
> I found "coroutine is not a callable error" on the Not Found(404)
responses on `/auth/fab/v1` endpo
Satoshi-Sh commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2010465008
After Jarek fixed the CI image, I'm able to see the same errors locally.
I found "coroutine is not a callable error" on the Not Found(404) responses
on `/auth/fab/v1` endpoin
potiuk commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-2004907580
As discussed on slack https://github.com/sudiptob2/airflow/pull/22 -> has
lkely solution (assets not compiled in the CI image).
You will also need to rebase and resolve conflicts
vincbeck commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1999885728
> > > breeze testing tests
tests/api_connexion/endpoints/test_connection_endpoint.py
> >
> >
> > It might be my environment that is wrongly set up ... When I run `breeze
tes
Satoshi-Sh commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1999794704
> > breeze testing tests
tests/api_connexion/endpoints/test_connection_endpoint.py
>
> It might be my environment that is wrongly set up ... When I run `breeze
testing tests t
vincbeck commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1997426495
> Apart form `ValueError: The name '/auth/fab/v1' is already registered for
a different blueprint. Use 'name=' to provide a unique name.` we also get the
following error which does not
sudiptob2 commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1996009442
Apart form `ValueError: The name '/auth/fab/v1' is already registered for a
different blueprint. Use 'name=' to provide a unique name.` we also get the
following error which does not
vincbeck commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1991985289
> breeze testing tests
tests/api_connexion/endpoints/test_connection_endpoint.py
It might be my environment that is wrongly set up ... When I run `breeze
testing tests tests/api
vincbeck commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1989411951
> I think that for a lot of the tests just need to creatively look at the
way how we initialize things
>
> ```
> ValueError: The name '/auth/fab/v1' is already registered for
potiuk commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1987109928
I think that for a lot of the tests just need to creatively look at the way
how we initialize things
```
ValueError: The name '/auth/fab/v1' is already registered for a differe
sudiptob2 commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1987085832
### Need_Help
We've made significant improvements to several unit tests. However, there
are still failing test cases. If anyone has any thoughts or pointers regarding
the failing t
Satoshi-Sh commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1977030307
I tried [another
approach](https://github.com/sudiptob2/airflow/pull/15#:~:text=Sh%20commented%20now-,Approach%20without%20using%20the%20Middleware,not%20sure%20if%20this%20approach%2
Satoshi-Sh commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1974919951
I have been working on `Taks 2 - Find a way to replace the Legacy
environ_overrides method from the old version of Connexion to fix the problem
in tests.` [here](https://github.com/s
potiuk commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1510034153
##
airflow/cli/commands/webserver_command.py:
##
@@ -383,7 +383,7 @@ def webserver(args):
"--workers",
str(num_workers),
"--work
Taragolis commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1510033350
##
airflow/cli/commands/webserver_command.py:
##
@@ -383,7 +383,7 @@ def webserver(args):
"--workers",
str(num_workers),
"--w
potiuk commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1510021644
##
airflow/cli/commands/webserver_command.py:
##
@@ -383,7 +383,7 @@ def webserver(args):
"--workers",
str(num_workers),
"--work
Taragolis commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1510018666
##
airflow/cli/commands/webserver_command.py:
##
@@ -383,7 +383,7 @@ def webserver(args):
"--workers",
str(num_workers),
"--w
potiuk commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1509903874
##
pyproject.toml:
##
@@ -83,7 +83,7 @@ dependencies = [
# The usage was added in #30596, seemingly only to override and improve
the default error message.
Satoshi-Sh commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1509874151
##
pyproject.toml:
##
@@ -83,7 +83,7 @@ dependencies = [
# The usage was added in #30596, seemingly only to override and improve
the default error message.
sudiptob2 commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1509401512
##
airflow/www/app.py:
##
@@ -70,7 +72,27 @@
def create_app(config=None, testing=False):
"""Create a new instance of Airflow WWW app."""
-flask_app = Fl
Satoshi-Sh commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1509407337
##
pyproject.toml:
##
@@ -83,7 +83,7 @@ dependencies = [
# The usage was added in #30596, seemingly only to override and improve
the default error message.
sudiptob2 commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1509401512
##
airflow/www/app.py:
##
@@ -70,7 +72,27 @@
def create_app(config=None, testing=False):
"""Create a new instance of Airflow WWW app."""
-flask_app = Fl
potiuk commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1509398492
##
airflow/www/app.py:
##
@@ -70,7 +72,27 @@
def create_app(config=None, testing=False):
"""Create a new instance of Airflow WWW app."""
-flask_app = Flask
potiuk commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1509377511
##
airflow/cli/commands/internal_api_command.py:
##
@@ -73,8 +73,8 @@ def internal_api(args):
log.info(f"Starting the Internal API server on port {args.port}
potiuk commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1509377511
##
airflow/cli/commands/internal_api_command.py:
##
@@ -73,8 +73,8 @@ def internal_api(args):
log.info(f"Starting the Internal API server on port {args.port}
potiuk commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1509371886
##
airflow/cli/commands/webserver_command.py:
##
@@ -383,7 +383,7 @@ def webserver(args):
"--workers",
str(num_workers),
"--work
potiuk commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1509367562
##
pyproject.toml:
##
@@ -83,7 +83,7 @@ dependencies = [
# The usage was added in #30596, seemingly only to override and improve
the default error message.
Taragolis commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1506618427
##
airflow/cli/commands/internal_api_command.py:
##
@@ -101,7 +101,7 @@ def internal_api(args):
"--workers",
str(num_workers),
potiuk commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1506602332
##
airflow/www/extensions/init_views.py:
##
@@ -323,11 +301,7 @@ def init_api_experimental(app):
app.extensions["csrf"].exempt(endpoints.api_experimental)
-de
Taragolis commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1969815804
> Also, please let me know what other unit tests are in the scope of this PR
apart from provider tests.
All broken tests should be fixed. We stick to `no broken window` policy
Taragolis commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1505734184
##
pyproject.toml:
##
@@ -83,7 +83,7 @@ dependencies = [
# The usage was added in #30596, seemingly only to override and improve
the default error message.
Satoshi-Sh commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1504752066
##
pyproject.toml:
##
@@ -83,7 +83,7 @@ dependencies = [
# The usage was added in #30596, seemingly only to override and improve
the default error message.
sudiptob2 commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1965433917
> The test `test_google_openid` is failing
@vincbeck
I investigated failing `test_google_openid ` These tests can be fixed by
just replacing `connexion_app` with `connexion_
sudiptob2 commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1965339552
> The test `test_google_openid` is failing
Thanks for the pointer, we will investigate this.
--
This is an automated message from the Apache Git Service.
To respond to the mes
vincbeck commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1964965577
The test `test_google_openid` is failing
--
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
vincbeck commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1503146149
##
airflow/www/extensions/init_views.py:
##
@@ -323,11 +301,7 @@ def init_api_experimental(app):
app.extensions["csrf"].exempt(endpoints.api_experimental)
-
Satoshi-Sh commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1503078047
##
pyproject.toml:
##
@@ -83,7 +83,7 @@ dependencies = [
# The usage was added in #30596, seemingly only to override and improve
the default error message.
Taragolis commented on code in PR #37638:
URL: https://github.com/apache/airflow/pull/37638#discussion_r1502453187
##
pyproject.toml:
##
@@ -83,7 +83,7 @@ dependencies = [
# The usage was added in #30596, seemingly only to override and improve
the default error message.
Satoshi-Sh commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1963111482
Oh, it worked now with the flag. I thought `export
AIRFLOW__CORE__LOAD_EXAMPLES=True` in the init.sh takes care of it.
Other than this `base_paths.append(blueprint.url_prefix
sudiptob2 commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1963094898
> Nicely done, @sudiptob2 . I checked it with `test_python_client.py`. The
first error is the same as before this update and the second one is just error
404 instead of the csrf issue
potiuk commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1963064558
> Can we get a demo/explanation session in the next meeting regarding how
the CSRF protection works in airflow, especially in the context of this PR?
Once I do a bit of homework o
potiuk commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1963064414
cc: @VladaZakharova - just adding you for awareness :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL a
Satoshi-Sh commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1963058925
Nicely done, @sudiptob2 . I checked it with `test_python_client.py`. The
first error is the same as before this update and the second one is just error
404 instead of the csrf issue
sudiptob2 commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1963045368
We might be able to handle `csrf` exemption logic in the following way.
```python3
@connexion_app.app.before_request
def before_request():
"""Exempts the vi
sudiptob2 commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1963024149
> Yes, it's scope. I have this now.
>
> ```
> # asgi-csrf skip_if_scope
> def skip_api_paths(scope):
> return scope["path"].startswith("/api/v1")
>
>
Satoshi-Sh commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1962965779
Yes, it's scope. I have this now.
```
# asgi-csrf skip_if_scope
def skip_api_paths(scope):
return scope["path"].startswith("/api/v1")
flask_app
potiuk commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1962894091
> What the score will be in the airflow project?
I assume the scope :) ?. I believe the scope is the base URL of Airflow
webserver (not 100% sure how asg-csrf does it but that's wh
Satoshi-Sh commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1962759936
## Problem
Due to the upgrading connextion v3, we cannot access blueprints( they moved
the blueprint registration code inside their codebase). We used the returned
blueprint to ma
sudiptob2 commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1962060087
### Refinement: Returning `flask_app` instead of `connexion_app`
Ref: https://github.com/apache/airflow/pull/37555#discussion_r1496006432
Mentions: @vincbeck
We investigated
vincbeck commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1961808501
> Hi @vincbeck
>
> We have defined the scope of **Task 1** in the PR description. Thank you
for your previous comments and suggestions to complete this task. There is a
part of
sudiptob2 commented on PR #37638:
URL: https://github.com/apache/airflow/pull/37638#issuecomment-1961739609
Hi @vincbeck
We have defined the scope of **Task 1** in the PR description. Thank you for
your previous comments and suggestions to complete this task. There is a part
of the
sudiptob2 opened a new pull request, #37638:
URL: https://github.com/apache/airflow/pull/37638
---
**^ Add meaningful description above**
Read the **[Pull Request
Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#
72 matches
Mail list logo
