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/03/15 16:38:39 UTC
[GitHub] [airflow] ashb opened a new pull request #14808: Rearange API auth tests to the correct place
ashb opened a new pull request #14808:
URL: https://github.com/apache/airflow/pull/14808
- tests/api/auth/bakcned/test_basic_auth was almost exclusively testing
the Connexion API -- moved in to tests/api_connexion/ folder.
(To make this work the conftest for connexion has been moved up a
folder out of .../endpoints/ )
- tests/test_utils/test_remote_user_api_auth_backend.py is testing test
code itself, and so isn't needed
- Created/copied Basic auth test to
tests/api_connexion/test_basic_auth.py -- this ensures the Connexion
API handles basic auth correctly too.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #14808: Rearange API auth tests to the correct place
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #14808:
URL: https://github.com/apache/airflow/pull/14808#discussion_r594499139
##########
File path: tests/api/auth/backend/test_basic_auth.py
##########
@@ -22,26 +22,26 @@
from parameterized import parameterized
from airflow.www.app import create_app
-from tests.test_utils.api_connexion_utils import assert_401
from tests.test_utils.config import conf_vars
from tests.test_utils.db import clear_db_pools
class TestBasicAuth(unittest.TestCase):
- def setUp(self) -> None:
+ @classmethod
+ def setUpClass(cls) -> None:
Review comment:
Drive-by speed up here too.
Will do a bigger optimization PR shortly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14808: Rearange API auth tests to the correct place
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14808:
URL: https://github.com/apache/airflow/pull/14808#issuecomment-799597146
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #14808: Rearange API auth tests to the correct place
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #14808:
URL: https://github.com/apache/airflow/pull/14808#discussion_r594500264
##########
File path: tests/test_utils/test_remote_user_api_auth_backend.py
##########
@@ -1,95 +0,0 @@
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements. See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership. The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License. You may obtain a copy of the License at
-#
-# http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing,
-# software distributed under the License is distributed on an
-# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-# KIND, either express or implied. See the License for the
-# specific language governing permissions and limitations
-# under the License.
-
-import unittest
-from unittest import mock
-
-from flask_login import current_user
-
-from airflow.www.app import create_app
-from tests.test_utils.config import conf_vars
-from tests.test_utils.db import clear_db_pools
-
-
-class TestGoogleOpenID(unittest.TestCase):
- def setUp(self) -> None:
- with conf_vars(
- {("api", "auth_backend"): "tests.test_utils.remote_user_api_auth_backend"}
Review comment:
The purpose of this test file appeared to be to test `tests.test_utils.remote_user_api_auth_backend`, but that is "exercised" by the other Connexion tests which use this API, so we don't need to test this again.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #14808: Rearange API auth tests to the correct place
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #14808:
URL: https://github.com/apache/airflow/pull/14808#discussion_r594499383
##########
File path: tests/api/auth/backend/test_basic_auth.py
##########
@@ -55,23 +55,10 @@ def test_success(self):
clear_db_pools()
with self.app.test_client() as test_client:
- response = test_client.get("/api/v1/pools", headers={"Authorization": token})
+ response = test_client.get("/api/experimental/pools", headers={"Authorization": token})
assert current_user.email == "test@fab.org"
assert response.status_code == 200
- assert response.json == {
Review comment:
We're not testing the API, so we don't care about the response, just the status code.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb merged pull request #14808: Rearange API auth tests to the correct place
Posted by GitBox <gi...@apache.org>.
ashb merged pull request #14808:
URL: https://github.com/apache/airflow/pull/14808
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org