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 2020/06/01 16:36:12 UTC
[GitHub] [airflow] OmairK opened a new pull request #9097: [WIP] Scheama and CRUD endpoints Pool
OmairK opened a new pull request #9097:
URL: https://github.com/apache/airflow/pull/9097
---
Make sure to mark the boxes below before creating PR: [x]
- [ ] Description above provides context of the change
- [ ] Unit tests coverage for changes (not needed for documentation changes)
- [ ] Target Github ISSUE in description if exists
- [ ] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [ ] Relevant documentation is updated including usage instructions.
- [ ] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
---
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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] ephraimbuddy commented on a change in pull request #9097: [WIP] Schema and CRUD endpoints Pool
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#discussion_r435798876
##########
File path: airflow/api_connexion/endpoints/pool_endpoint.py
##########
@@ -26,18 +30,33 @@ def delete_pool():
raise NotImplementedError("Not implemented yet.")
-def get_pool():
+@provide_session
+def get_pool(pool_name, session):
"""
Get a pool
"""
- raise NotImplementedError("Not implemented yet.")
+ pool_id = pool_name
+ query = session.query(Pool)
+ pool = query.filter(Pool.pool == pool_id).one_or_none()
+
+ if pool is None:
+ raise NotFound("Pool not found")
+ return pool_schema.dump(pool)
-def get_pools():
+@provide_session
+def get_pools(session):
"""
Get all pools
"""
- raise NotImplementedError("Not implemented yet.")
+ offset = request.args.get(parameters.page_offset, 0)
+ limit = min(request.args.get(parameters.page_limit, 100), 100)
Review comment:
You can add tests to handle pagination 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] OmairK commented on a change in pull request #9097: [WIP] Schema and CRUD endpoints Pool
Posted by GitBox <gi...@apache.org>.
OmairK commented on a change in pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#discussion_r435454718
##########
File path: airflow/api_connexion/exceptions.py
##########
@@ -0,0 +1,66 @@
+# 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.
+
+
+def response_404(
Review comment:
Thank you this is exactly what I was stuck on.
----------------------------------------------------------------
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] mik-laj merged pull request #9097: Schema and read endpoints Pool
Posted by GitBox <gi...@apache.org>.
mik-laj merged pull request #9097:
URL: https://github.com/apache/airflow/pull/9097
----------------------------------------------------------------
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] OmairK commented on a change in pull request #9097: [WIP] Schema and CRUD endpoints Pool
Posted by GitBox <gi...@apache.org>.
OmairK commented on a change in pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#discussion_r435454718
##########
File path: airflow/api_connexion/exceptions.py
##########
@@ -0,0 +1,66 @@
+# 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.
+
+
+def response_404(
Review comment:
Thank you, this exactly what I was stuck on.
> [](https://termbin.com/legq)
> To add this commit to your brach, run the following command:
>
> ```
> curl https://termbin.com/legq | git am
> ```
Thank you this is exactly what I was stuck on.
----------------------------------------------------------------
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] OmairK edited a comment on pull request #9097: Schema and read endpoints Pool
Posted by GitBox <gi...@apache.org>.
OmairK edited a comment on pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#issuecomment-644089496
> Github Action is sad. I think these are a flaky tests. However, to be sure, could you do a rebase? Over 3 red crosses are not good.
All the api_connexion tests are passing now.
Just one test is failing
> tests/www/test_views.py
----------------------------------------------------------------
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] OmairK commented on a change in pull request #9097: [WIP] Schema and CRUD endpoints Pool
Posted by GitBox <gi...@apache.org>.
OmairK commented on a change in pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#discussion_r438893729
##########
File path: tests/api_connexion/schemas/test_pool_schemas.py
##########
@@ -0,0 +1,89 @@
+# 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 airflow.api_connexion.schemas.pool_schema import PoolCollection, pool_collection_schema, pool_schema
+from airflow.models.pool import Pool
+from airflow.utils.session import provide_session
+from tests.test_utils.db import clear_db_pools
+
+
+class TestPoolSchema(unittest.TestCase):
+ def setUp(self) -> None:
+ clear_db_pools()
+
+ def tearDown(self) -> None:
+ clear_db_pools()
+
+ @provide_session
+ def test_serialize(self, session):
+ pool_model = Pool(pool='test_pool', slots=2)
+ session.add(pool_model)
+ session.commit()
+ pool_instance = session.query(Pool).filter(
+ Pool.pool == pool_model.pool).first()
+ serialized_pool = pool_schema.dump(pool_instance)
+ self.assertEqual(
+ serialized_pool.data,
+ {
+ "name": "test_pool",
+ "slots": 2,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 2,
+ }
+ )
+
+ @provide_session
+ def test_desearialize(self, session):
+ pool_dict = {"name": "test_pool", "slots": 3}
+ deserialized_pool = pool_schema.load(pool_dict, session=session)
+ self.assertIsInstance(deserialized_pool.data, Pool)
+
+
+class TestPoolCollectionSchema(unittest.TestCase):
+ def setUp(self) -> None:
+ clear_db_pools()
+
+ def tearDown(self) -> None:
+ clear_db_pools()
+
+ def test_serialize(self):
+ pool_model_a = Pool(pool="test_pool_a", slots=3)
+ pool_model_b = Pool(pool="test_pool_b", slots=3)
+ instance = PoolCollection(
+ pools=[pool_model_a, pool_model_b], total_entries=2)
+ self.assertEqual(
+ {"pools": [{"name": "test_pool_a",
Review comment:
Here is the change `98ecf3b`
----------------------------------------------------------------
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] mik-laj commented on a change in pull request #9097: [WIP] Schema and CRUD endpoints Pool
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#discussion_r435453557
##########
File path: airflow/api_connexion/exceptions.py
##########
@@ -0,0 +1,66 @@
+# 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.
+
+
+def response_404(
Review comment:
I think, we should use exception to handle errors.
I have prepared an example that shows how to do it.
https://termbin.com/legq
To add this commit to your brach, run the following command:
```
curl https://termbin.com/legq | git am
```
----------------------------------------------------------------
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] OmairK commented on a change in pull request #9097: [WIP] Schema and CRUD endpoints Pool
Posted by GitBox <gi...@apache.org>.
OmairK commented on a change in pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#discussion_r435620216
##########
File path: airflow/api_connexion/exceptions.py
##########
@@ -0,0 +1,66 @@
+# 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.
+
+
+def response_404(
Review comment:
Fixed `00da53d `
----------------------------------------------------------------
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] mik-laj commented on pull request #9097: Schema and read endpoints Pool
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#issuecomment-644171644
@OmairK Can you do a rebase? We fixed test_views.py. https://github.com/apache/airflow/pull/9307
----------------------------------------------------------------
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] OmairK commented on a change in pull request #9097: [WIP] Schema and CRUD endpoints Pool
Posted by GitBox <gi...@apache.org>.
OmairK commented on a change in pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#discussion_r438871697
##########
File path: tests/api_connexion/schemas/test_pool_schemas.py
##########
@@ -0,0 +1,89 @@
+# 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 airflow.api_connexion.schemas.pool_schema import PoolCollection, pool_collection_schema, pool_schema
+from airflow.models.pool import Pool
+from airflow.utils.session import provide_session
+from tests.test_utils.db import clear_db_pools
+
+
+class TestPoolSchema(unittest.TestCase):
+ def setUp(self) -> None:
+ clear_db_pools()
+
+ def tearDown(self) -> None:
+ clear_db_pools()
+
+ @provide_session
+ def test_serialize(self, session):
+ pool_model = Pool(pool='test_pool', slots=2)
+ session.add(pool_model)
+ session.commit()
+ pool_instance = session.query(Pool).filter(
+ Pool.pool == pool_model.pool).first()
+ serialized_pool = pool_schema.dump(pool_instance)
+ self.assertEqual(
+ serialized_pool.data,
+ {
+ "name": "test_pool",
+ "slots": 2,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 2,
+ }
+ )
+
+ @provide_session
+ def test_desearialize(self, session):
+ pool_dict = {"name": "test_pool", "slots": 3}
+ deserialized_pool = pool_schema.load(pool_dict, session=session)
+ self.assertIsInstance(deserialized_pool.data, Pool)
+
+
+class TestPoolCollectionSchema(unittest.TestCase):
+ def setUp(self) -> None:
+ clear_db_pools()
+
+ def tearDown(self) -> None:
+ clear_db_pools()
+
+ def test_serialize(self):
+ pool_model_a = Pool(pool="test_pool_a", slots=3)
+ pool_model_b = Pool(pool="test_pool_b", slots=3)
+ instance = PoolCollection(
+ pools=[pool_model_a, pool_model_b], total_entries=2)
+ self.assertEqual(
+ {"pools": [{"name": "test_pool_a",
Review comment:
Sure, I was using autopep8 from the start I will use black from now on.
----------------------------------------------------------------
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] OmairK commented on a change in pull request #9097: [WIP] Schema and CRUD endpoints Pool
Posted by GitBox <gi...@apache.org>.
OmairK commented on a change in pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#discussion_r435840776
##########
File path: airflow/api_connexion/endpoints/pool_endpoint.py
##########
@@ -26,18 +30,33 @@ def delete_pool():
raise NotImplementedError("Not implemented yet.")
-def get_pool():
+@provide_session
+def get_pool(pool_name, session):
"""
Get a pool
"""
- raise NotImplementedError("Not implemented yet.")
+ pool_id = pool_name
+ query = session.query(Pool)
+ pool = query.filter(Pool.pool == pool_id).one_or_none()
+
+ if pool is None:
+ raise NotFound("Pool not found")
+ return pool_schema.dump(pool)
-def get_pools():
+@provide_session
+def get_pools(session):
"""
Get all pools
"""
- raise NotImplementedError("Not implemented yet.")
+ offset = request.args.get(parameters.page_offset, 0)
+ limit = min(request.args.get(parameters.page_limit, 100), 100)
Review comment:
Thanks
Here is the change `983e673`
----------------------------------------------------------------
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] mik-laj commented on pull request #9097: Schema and read endpoints Pool
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#issuecomment-643836173
Github Action is sad. I think these are a flaky tests. However, to be sure, could you do a rebase? Over 3 red crosses are not good.
----------------------------------------------------------------
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] mik-laj commented on a change in pull request #9097: [WIP] Schema and CRUD endpoints Pool
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#discussion_r438865487
##########
File path: tests/api_connexion/schemas/test_pool_schemas.py
##########
@@ -0,0 +1,89 @@
+# 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 airflow.api_connexion.schemas.pool_schema import PoolCollection, pool_collection_schema, pool_schema
+from airflow.models.pool import Pool
+from airflow.utils.session import provide_session
+from tests.test_utils.db import clear_db_pools
+
+
+class TestPoolSchema(unittest.TestCase):
+ def setUp(self) -> None:
+ clear_db_pools()
+
+ def tearDown(self) -> None:
+ clear_db_pools()
+
+ @provide_session
+ def test_serialize(self, session):
+ pool_model = Pool(pool='test_pool', slots=2)
+ session.add(pool_model)
+ session.commit()
+ pool_instance = session.query(Pool).filter(
+ Pool.pool == pool_model.pool).first()
+ serialized_pool = pool_schema.dump(pool_instance)
+ self.assertEqual(
+ serialized_pool.data,
+ {
+ "name": "test_pool",
+ "slots": 2,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 2,
+ }
+ )
+
+ @provide_session
+ def test_desearialize(self, session):
+ pool_dict = {"name": "test_pool", "slots": 3}
+ deserialized_pool = pool_schema.load(pool_dict, session=session)
+ self.assertIsInstance(deserialized_pool.data, Pool)
+
+
+class TestPoolCollectionSchema(unittest.TestCase):
+ def setUp(self) -> None:
+ clear_db_pools()
+
+ def tearDown(self) -> None:
+ clear_db_pools()
+
+ def test_serialize(self):
+ pool_model_a = Pool(pool="test_pool_a", slots=3)
+ pool_model_b = Pool(pool="test_pool_b", slots=3)
+ instance = PoolCollection(
+ pools=[pool_model_a, pool_model_b], total_entries=2)
+ self.assertEqual(
+ {"pools": [{"name": "test_pool_a",
Review comment:
Can you format his file with black? This source code snippet looks weird.
----------------------------------------------------------------
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] OmairK commented on a change in pull request #9097: [WIP] Schema and CRUD endpoints Pool
Posted by GitBox <gi...@apache.org>.
OmairK commented on a change in pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#discussion_r437471258
##########
File path: airflow/api_connexion/endpoints/pool_endpoint.py
##########
@@ -26,18 +30,33 @@ def delete_pool():
raise NotImplementedError("Not implemented yet.")
-def get_pool():
+@provide_session
+def get_pool(pool_name, session):
"""
Get a pool
"""
- raise NotImplementedError("Not implemented yet.")
+ pool_id = pool_name
+ query = session.query(Pool)
+ pool = query.filter(Pool.pool == pool_id).one_or_none()
+
+ if pool is None:
+ raise NotFound("Pool not found")
+ return pool_schema.dump(pool)
-def get_pools():
+@provide_session
+def get_pools(session):
"""
Get all pools
"""
- raise NotImplementedError("Not implemented yet.")
+ offset = request.args.get(parameters.page_offset, 0)
+ limit = min(int(request.args.get(parameters.page_limit, 100)), 100)
+
+ query = session.query(Pool)
+ query = query.offset(offset).limit(limit)
+
+ pools = query.all()
+ return pool_collection_schema.dump(PoolCollection(pools=pools, total_entries=len(pools))).data
Review comment:
Thanks.
Here is the change `ce9b79f`
##########
File path: tests/api_connexion/endpoints/test_pool_endpoint.py
##########
@@ -16,51 +16,143 @@
# under the License.
import unittest
-import pytest
-
+from airflow.models.pool import Pool
+from airflow.utils.session import provide_session
from airflow.www import app
+from tests.test_utils.db import clear_db_pools
-class TestPoolEndpoint(unittest.TestCase):
+class TestBasePoolEndpoints(unittest.TestCase):
@classmethod
def setUpClass(cls) -> None:
super().setUpClass()
cls.app = app.create_app(testing=True) # type:ignore
def setUp(self) -> None:
self.client = self.app.test_client() # type:ignore
+ super().setUp()
+ clear_db_pools()
-
-class TestDeletePool(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.delete("/api/v1/pools/TEST_POOL_NAME")
- assert response.status_code == 204
+ def tearDown(self) -> None:
+ clear_db_pools()
-class TestGetPool(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.get("/api/v1/pools/TEST_POOL_NAME")
+class TestGetPools(TestBasePoolEndpoints):
+ @provide_session
+ def test_response_200(self, session):
+ pool_model = Pool(pool="test_pool_a", slots=3)
+ session.add(pool_model)
+ session.commit()
+ result = session.query(Pool).all()
+ assert len(result) == 2 # accounts for the default pool as well
+ response = self.client.get("/api/v1/pools")
assert response.status_code == 200
+ self.assertEqual(
+ {"pools": [{"name": "default_pool",
+ "slots": 128,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 128,
+ },
+ {"name": "test_pool_a",
+ "slots": 3,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 3,
+ },
+ ],
+ "total_entries": 2,
+ },
+ response.json,
+ )
-
-class TestGetPools(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.get("/api/v1/pools")
+ @provide_session
+ def test_limit(self, session):
+ pools = [Pool(pool=f"test_pool{i}", slots=1) for i in range(120)]
+ session.add_all(pools)
+ session.commit()
+ result = session.query(Pool).count()
+ self.assertEqual(result, 121) # accounts for default pool as well
+ # Test for limit under 100
+ response = self.client.get("/api/v1/pools?limit=10")
+ assert response.status_code == 200
+ self.assertEqual(response.json.get("total_entries"), 10)
Review comment:
Here is the change ce9b79f
##########
File path: tests/api_connexion/endpoints/test_pool_endpoint.py
##########
@@ -16,51 +16,143 @@
# under the License.
import unittest
-import pytest
-
+from airflow.models.pool import Pool
+from airflow.utils.session import provide_session
from airflow.www import app
+from tests.test_utils.db import clear_db_pools
-class TestPoolEndpoint(unittest.TestCase):
+class TestBasePoolEndpoints(unittest.TestCase):
@classmethod
def setUpClass(cls) -> None:
super().setUpClass()
cls.app = app.create_app(testing=True) # type:ignore
def setUp(self) -> None:
self.client = self.app.test_client() # type:ignore
+ super().setUp()
+ clear_db_pools()
-
-class TestDeletePool(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.delete("/api/v1/pools/TEST_POOL_NAME")
- assert response.status_code == 204
+ def tearDown(self) -> None:
+ clear_db_pools()
-class TestGetPool(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.get("/api/v1/pools/TEST_POOL_NAME")
+class TestGetPools(TestBasePoolEndpoints):
+ @provide_session
+ def test_response_200(self, session):
+ pool_model = Pool(pool="test_pool_a", slots=3)
+ session.add(pool_model)
+ session.commit()
+ result = session.query(Pool).all()
+ assert len(result) == 2 # accounts for the default pool as well
+ response = self.client.get("/api/v1/pools")
assert response.status_code == 200
+ self.assertEqual(
+ {"pools": [{"name": "default_pool",
+ "slots": 128,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 128,
+ },
+ {"name": "test_pool_a",
+ "slots": 3,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 3,
+ },
+ ],
+ "total_entries": 2,
+ },
+ response.json,
+ )
-
-class TestGetPools(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.get("/api/v1/pools")
+ @provide_session
+ def test_limit(self, session):
+ pools = [Pool(pool=f"test_pool{i}", slots=1) for i in range(120)]
+ session.add_all(pools)
+ session.commit()
+ result = session.query(Pool).count()
+ self.assertEqual(result, 121) # accounts for default pool as well
+ # Test for limit under 100
+ response = self.client.get("/api/v1/pools?limit=10")
+ assert response.status_code == 200
+ self.assertEqual(response.json.get("total_entries"), 10)
+ # Test for limit over 100
+ response = self.client.get("/api/v1/pools?limit=110")
assert response.status_code == 200
+ self.assertEqual(response.json.get('total_entries'), 100)
Review comment:
Here is the change ce9b79f
##########
File path: tests/api_connexion/endpoints/test_pool_endpoint.py
##########
@@ -16,51 +16,143 @@
# under the License.
import unittest
-import pytest
-
+from airflow.models.pool import Pool
+from airflow.utils.session import provide_session
from airflow.www import app
+from tests.test_utils.db import clear_db_pools
-class TestPoolEndpoint(unittest.TestCase):
+class TestBasePoolEndpoints(unittest.TestCase):
@classmethod
def setUpClass(cls) -> None:
super().setUpClass()
cls.app = app.create_app(testing=True) # type:ignore
def setUp(self) -> None:
self.client = self.app.test_client() # type:ignore
+ super().setUp()
+ clear_db_pools()
-
-class TestDeletePool(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.delete("/api/v1/pools/TEST_POOL_NAME")
- assert response.status_code == 204
+ def tearDown(self) -> None:
+ clear_db_pools()
-class TestGetPool(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.get("/api/v1/pools/TEST_POOL_NAME")
+class TestGetPools(TestBasePoolEndpoints):
+ @provide_session
+ def test_response_200(self, session):
+ pool_model = Pool(pool="test_pool_a", slots=3)
+ session.add(pool_model)
+ session.commit()
+ result = session.query(Pool).all()
+ assert len(result) == 2 # accounts for the default pool as well
+ response = self.client.get("/api/v1/pools")
assert response.status_code == 200
+ self.assertEqual(
+ {"pools": [{"name": "default_pool",
+ "slots": 128,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 128,
+ },
+ {"name": "test_pool_a",
+ "slots": 3,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 3,
+ },
+ ],
+ "total_entries": 2,
+ },
+ response.json,
+ )
-
-class TestGetPools(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.get("/api/v1/pools")
+ @provide_session
+ def test_limit(self, session):
+ pools = [Pool(pool=f"test_pool{i}", slots=1) for i in range(120)]
+ session.add_all(pools)
+ session.commit()
+ result = session.query(Pool).count()
+ self.assertEqual(result, 121) # accounts for default pool as well
+ # Test for limit under 100
+ response = self.client.get("/api/v1/pools?limit=10")
+ assert response.status_code == 200
+ self.assertEqual(response.json.get("total_entries"), 10)
+ # Test for limit over 100
+ response = self.client.get("/api/v1/pools?limit=110")
assert response.status_code == 200
+ self.assertEqual(response.json.get('total_entries'), 100)
Review comment:
Here is the change ~ce9b79f~ 36d3a3f
##########
File path: tests/api_connexion/endpoints/test_pool_endpoint.py
##########
@@ -16,51 +16,143 @@
# under the License.
import unittest
-import pytest
-
+from airflow.models.pool import Pool
+from airflow.utils.session import provide_session
from airflow.www import app
+from tests.test_utils.db import clear_db_pools
-class TestPoolEndpoint(unittest.TestCase):
+class TestBasePoolEndpoints(unittest.TestCase):
@classmethod
def setUpClass(cls) -> None:
super().setUpClass()
cls.app = app.create_app(testing=True) # type:ignore
def setUp(self) -> None:
self.client = self.app.test_client() # type:ignore
+ super().setUp()
+ clear_db_pools()
-
-class TestDeletePool(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.delete("/api/v1/pools/TEST_POOL_NAME")
- assert response.status_code == 204
+ def tearDown(self) -> None:
+ clear_db_pools()
-class TestGetPool(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.get("/api/v1/pools/TEST_POOL_NAME")
+class TestGetPools(TestBasePoolEndpoints):
+ @provide_session
+ def test_response_200(self, session):
+ pool_model = Pool(pool="test_pool_a", slots=3)
+ session.add(pool_model)
+ session.commit()
+ result = session.query(Pool).all()
+ assert len(result) == 2 # accounts for the default pool as well
+ response = self.client.get("/api/v1/pools")
assert response.status_code == 200
+ self.assertEqual(
+ {"pools": [{"name": "default_pool",
+ "slots": 128,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 128,
+ },
+ {"name": "test_pool_a",
+ "slots": 3,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 3,
+ },
+ ],
+ "total_entries": 2,
+ },
+ response.json,
+ )
-
-class TestGetPools(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.get("/api/v1/pools")
+ @provide_session
+ def test_limit(self, session):
+ pools = [Pool(pool=f"test_pool{i}", slots=1) for i in range(120)]
+ session.add_all(pools)
+ session.commit()
+ result = session.query(Pool).count()
+ self.assertEqual(result, 121) # accounts for default pool as well
+ # Test for limit under 100
+ response = self.client.get("/api/v1/pools?limit=10")
+ assert response.status_code == 200
+ self.assertEqual(response.json.get("total_entries"), 10)
Review comment:
Here is the change ~ce9b79f~ 36d3a3f
##########
File path: airflow/api_connexion/endpoints/pool_endpoint.py
##########
@@ -26,18 +30,33 @@ def delete_pool():
raise NotImplementedError("Not implemented yet.")
-def get_pool():
+@provide_session
+def get_pool(pool_name, session):
"""
Get a pool
"""
- raise NotImplementedError("Not implemented yet.")
+ pool_id = pool_name
+ query = session.query(Pool)
+ pool = query.filter(Pool.pool == pool_id).one_or_none()
+
+ if pool is None:
+ raise NotFound("Pool not found")
+ return pool_schema.dump(pool)
-def get_pools():
+@provide_session
+def get_pools(session):
"""
Get all pools
"""
- raise NotImplementedError("Not implemented yet.")
+ offset = request.args.get(parameters.page_offset, 0)
+ limit = min(int(request.args.get(parameters.page_limit, 100)), 100)
+
+ query = session.query(Pool)
+ query = query.offset(offset).limit(limit)
+
+ pools = query.all()
+ return pool_collection_schema.dump(PoolCollection(pools=pools, total_entries=len(pools))).data
Review comment:
Thanks.
Here is the change ~ce9b79f~ 36d3a3f
----------------------------------------------------------------
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] ephraimbuddy commented on a change in pull request #9097: [WIP] Schema and CRUD endpoints Pool
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#discussion_r435798377
##########
File path: airflow/api_connexion/endpoints/pool_endpoint.py
##########
@@ -26,18 +30,33 @@ def delete_pool():
raise NotImplementedError("Not implemented yet.")
-def get_pool():
+@provide_session
+def get_pool(pool_name, session):
"""
Get a pool
"""
- raise NotImplementedError("Not implemented yet.")
+ pool_id = pool_name
+ query = session.query(Pool)
+ pool = query.filter(Pool.pool == pool_id).one_or_none()
+
+ if pool is None:
+ raise NotFound("Pool not found")
+ return pool_schema.dump(pool)
-def get_pools():
+@provide_session
+def get_pools(session):
"""
Get all pools
"""
- raise NotImplementedError("Not implemented yet.")
+ offset = request.args.get(parameters.page_offset, 0)
+ limit = min(request.args.get(parameters.page_limit, 100), 100)
Review comment:
```suggestion
limit = min(int(request.args.get(parameters.page_limit, 100)), 100)
```
----------------------------------------------------------------
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] OmairK commented on a change in pull request #9097: [WIP] Schema and CRUD endpoints Pool
Posted by GitBox <gi...@apache.org>.
OmairK commented on a change in pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#discussion_r435620626
##########
File path: airflow/api_connexion/exceptions.py
##########
@@ -0,0 +1,66 @@
+# 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.
+
+
+def response_404(
Review comment:
Added other http exception classes `cd126b7`
----------------------------------------------------------------
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] mik-laj commented on a change in pull request #9097: [WIP] Schema and CRUD endpoints Pool
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#discussion_r437017632
##########
File path: airflow/api_connexion/endpoints/pool_endpoint.py
##########
@@ -26,18 +30,33 @@ def delete_pool():
raise NotImplementedError("Not implemented yet.")
-def get_pool():
+@provide_session
+def get_pool(pool_name, session):
"""
Get a pool
"""
- raise NotImplementedError("Not implemented yet.")
+ pool_id = pool_name
+ query = session.query(Pool)
+ pool = query.filter(Pool.pool == pool_id).one_or_none()
+
+ if pool is None:
+ raise NotFound("Pool not found")
+ return pool_schema.dump(pool)
-def get_pools():
+@provide_session
+def get_pools(session):
"""
Get all pools
"""
- raise NotImplementedError("Not implemented yet.")
+ offset = request.args.get(parameters.page_offset, 0)
+ limit = min(int(request.args.get(parameters.page_limit, 100)), 100)
+
+ query = session.query(Pool)
+ query = query.offset(offset).limit(limit)
+
+ pools = query.all()
+ return pool_collection_schema.dump(PoolCollection(pools=pools, total_entries=len(pools))).data
Review comment:
Total entries should contain the number of all items, not just the number of items on the current page.
##########
File path: tests/api_connexion/endpoints/test_pool_endpoint.py
##########
@@ -16,51 +16,143 @@
# under the License.
import unittest
-import pytest
-
+from airflow.models.pool import Pool
+from airflow.utils.session import provide_session
from airflow.www import app
+from tests.test_utils.db import clear_db_pools
-class TestPoolEndpoint(unittest.TestCase):
+class TestBasePoolEndpoints(unittest.TestCase):
@classmethod
def setUpClass(cls) -> None:
super().setUpClass()
cls.app = app.create_app(testing=True) # type:ignore
def setUp(self) -> None:
self.client = self.app.test_client() # type:ignore
+ super().setUp()
+ clear_db_pools()
-
-class TestDeletePool(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.delete("/api/v1/pools/TEST_POOL_NAME")
- assert response.status_code == 204
+ def tearDown(self) -> None:
+ clear_db_pools()
-class TestGetPool(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.get("/api/v1/pools/TEST_POOL_NAME")
+class TestGetPools(TestBasePoolEndpoints):
+ @provide_session
+ def test_response_200(self, session):
+ pool_model = Pool(pool="test_pool_a", slots=3)
+ session.add(pool_model)
+ session.commit()
+ result = session.query(Pool).all()
+ assert len(result) == 2 # accounts for the default pool as well
+ response = self.client.get("/api/v1/pools")
assert response.status_code == 200
+ self.assertEqual(
+ {"pools": [{"name": "default_pool",
+ "slots": 128,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 128,
+ },
+ {"name": "test_pool_a",
+ "slots": 3,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 3,
+ },
+ ],
+ "total_entries": 2,
+ },
+ response.json,
+ )
-
-class TestGetPools(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.get("/api/v1/pools")
+ @provide_session
+ def test_limit(self, session):
+ pools = [Pool(pool=f"test_pool{i}", slots=1) for i in range(120)]
+ session.add_all(pools)
+ session.commit()
+ result = session.query(Pool).count()
+ self.assertEqual(result, 121) # accounts for default pool as well
+ # Test for limit under 100
+ response = self.client.get("/api/v1/pools?limit=10")
+ assert response.status_code == 200
+ self.assertEqual(response.json.get("total_entries"), 10)
Review comment:
```suggestion
self.assertEqual(response.json["total_entries"], 121)
```
It should return count of all items.
##########
File path: tests/api_connexion/endpoints/test_pool_endpoint.py
##########
@@ -16,51 +16,143 @@
# under the License.
import unittest
-import pytest
-
+from airflow.models.pool import Pool
+from airflow.utils.session import provide_session
from airflow.www import app
+from tests.test_utils.db import clear_db_pools
-class TestPoolEndpoint(unittest.TestCase):
+class TestBasePoolEndpoints(unittest.TestCase):
@classmethod
def setUpClass(cls) -> None:
super().setUpClass()
cls.app = app.create_app(testing=True) # type:ignore
def setUp(self) -> None:
self.client = self.app.test_client() # type:ignore
+ super().setUp()
+ clear_db_pools()
-
-class TestDeletePool(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.delete("/api/v1/pools/TEST_POOL_NAME")
- assert response.status_code == 204
+ def tearDown(self) -> None:
+ clear_db_pools()
-class TestGetPool(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.get("/api/v1/pools/TEST_POOL_NAME")
+class TestGetPools(TestBasePoolEndpoints):
+ @provide_session
+ def test_response_200(self, session):
+ pool_model = Pool(pool="test_pool_a", slots=3)
+ session.add(pool_model)
+ session.commit()
+ result = session.query(Pool).all()
+ assert len(result) == 2 # accounts for the default pool as well
+ response = self.client.get("/api/v1/pools")
assert response.status_code == 200
+ self.assertEqual(
+ {"pools": [{"name": "default_pool",
+ "slots": 128,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 128,
+ },
+ {"name": "test_pool_a",
+ "slots": 3,
+ "occupied_slots": 0,
+ "running_slots": 0,
+ "queued_slots": 0,
+ "open_slots": 3,
+ },
+ ],
+ "total_entries": 2,
+ },
+ response.json,
+ )
-
-class TestGetPools(TestPoolEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.get("/api/v1/pools")
+ @provide_session
+ def test_limit(self, session):
+ pools = [Pool(pool=f"test_pool{i}", slots=1) for i in range(120)]
+ session.add_all(pools)
+ session.commit()
+ result = session.query(Pool).count()
+ self.assertEqual(result, 121) # accounts for default pool as well
+ # Test for limit under 100
+ response = self.client.get("/api/v1/pools?limit=10")
+ assert response.status_code == 200
+ self.assertEqual(response.json.get("total_entries"), 10)
+ # Test for limit over 100
+ response = self.client.get("/api/v1/pools?limit=110")
assert response.status_code == 200
+ self.assertEqual(response.json.get('total_entries'), 100)
Review comment:
```suggestion
self.assertEqual(response.json['total_entries'], 121)
```
It should return the count of all items.
----------------------------------------------------------------
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] OmairK commented on pull request #9097: Schema and read endpoints Pool
Posted by GitBox <gi...@apache.org>.
OmairK commented on pull request #9097:
URL: https://github.com/apache/airflow/pull/9097#issuecomment-644089496
> Github Action is sad. I think these are a flaky tests. However, to be sure, could you do a rebase? Over 3 red crosses are not good.
All the api_connexion tests are passing.
Just one test is failing
> tests/www/test_views.py
----------------------------------------------------------------
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