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 14:54:51 UTC
[GitHub] [airflow] ephraimbuddy opened a new pull request #9095: [WIP] Add connection schema
ephraimbuddy opened a new pull request #9095:
URL: https://github.com/apache/airflow/pull/9095
---
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 #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r435477586
##########
File path: airflow/api_connexion/schemas/base_schema.py
##########
@@ -0,0 +1,79 @@
+# 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.
+from typing import List, Optional
+
+from marshmallow import post_dump
+from marshmallow_sqlalchemy import SQLAlchemySchema
+
+from airflow.exceptions import AirflowException
+
+
+class BaseSchema(SQLAlchemySchema):
+
+ """ Base Schema for sqlalchemy models
+
+ :param COLLECTION_NAME: A name to use to return serialized data if the data is a list
+ :type COLLECTION_NAME: str
+
+ :param FIELDS_FROM_NONE_TO_EMPTY_STRING: A list of fields to convert to empty string if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str]
+
+ :param FIELDS_FROM_NONE_TO_ZERO: A list of fields to convert to integer zero if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_ZERO: List[str]
+ """
+
+ COLLECTION_NAME: Optional[str] = None
+ FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str] = []
+ FIELDS_FROM_NONE_TO_ZERO: List[str] = []
+
+ def check_collection_name(self):
+ """
+ Method to check that COLLLECTION_NAME attribute is not None
+ """
+ if not self.COLLECTION_NAME:
+ raise AirflowException("The COLLECTION_NAME attribute is missing in the schema")
+
+ @post_dump(pass_many=True)
+ def wrap_with_envelope(self, data, many, **kwargs):
+ """
+ Checks if data is a list and use the envelope key to return it together
+ with total_entries meta
+ :param data: The deserialized data
+ :param many: Whether the data is a collection
+ """
+ self.check_collection_name()
+ if many:
+ data = self._process_list_data(data)
+ return {self.COLLECTION_NAME: data, 'total_entries': len(data)}
Review comment:
I have not tried pagination. Let me write a testcase for 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r434431185
##########
File path: airflow/api_connexion/schemas/base_schema.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.
+from typing import List, Optional
+
+from marshmallow import post_dump
+from marshmallow_sqlalchemy import SQLAlchemySchema
+
+from airflow.exceptions import AirflowException
+
+
+class BaseSchema(SQLAlchemySchema):
+
+ """ Base Schema for sqlalchemy models
+
+ :param COLLECTION_NAME: A name to use to return serialized data if the data is a list
+ :type COLLECTION_NAME: str
+
+ :param FIELDS_FROM_NONE_TO_EMPTY_STRING: A list of fields to convert to empty string if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str]
+
+ :param FIELDS_FROM_NONE_TO_ZERO: A list of fields to convert to integer zero if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_ZERO: List[str]
+ """
+
+ COLLECTION_NAME: Optional[str] = None
+ FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str] = []
+ FIELDS_FROM_NONE_TO_ZERO: List[str] = []
+
+ def check_collection_name(self):
+ """
+ Method to check that COLLLECTION_NAME attribute is not None
+ """
+ if not self.COLLECTION_NAME:
+ raise AirflowException("The COLLECTION_NAME attribute is missing in the schema")
+
+ @post_dump(pass_many=True)
+ def wrap_with_envelope(self, data, many, **kwargs):
+ """
+ Checks if data is a list and use the envelope key to return it together
+ with total_entries meta
+ :param data: The deserialized data
+ :param many: Whether the data is a collection
+ """
+ self.check_collection_name()
+ if many:
+ data = self._process_list_data(data)
+ return {self.COLLECTION_NAME: data, 'total_entries': len(data)}
+ data = self._process_data(data)
+ return data
+
+ def _process_list_data(self, data):
+ d_data = {}
+ d_list = []
+ for item in data:
+ for k, v in item.items():
+ if v is None and k in self.FIELDS_FROM_NONE_TO_EMPTY_STRING:
+ v = ''
+ elif v is None and k in self.FIELDS_FROM_NONE_TO_ZERO:
+ v = 0
+ d_data[k] = v
+ d_list.append(d_data)
+ d_data = {}
Review comment:
```suggestion
for item in data:
d_data = self._process_data(item)
d_list.append(d_data)
```
The code is duplicated below so this should work. WDYT?
----------------------------------------------------------------
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] turbaszek commented on a change in pull request #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r434428048
##########
File path: airflow/api_connexion/endpoints/connection_endpoint.py
##########
@@ -47,7 +65,7 @@ def patch_connection():
raise NotImplementedError("Not implemented yet.")
-def post_connection():
+def post_connection(session):
Review comment:
Nit: this is not related to read-only endpoints
----------------------------------------------------------------
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 #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r435504782
##########
File path: airflow/api_connexion/schemas/base_schema.py
##########
@@ -0,0 +1,79 @@
+# 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.
+from typing import List, Optional
+
+from marshmallow import post_dump
+from marshmallow_sqlalchemy import SQLAlchemySchema
+
+from airflow.exceptions import AirflowException
+
+
+class BaseSchema(SQLAlchemySchema):
+
+ """ Base Schema for sqlalchemy models
+
+ :param COLLECTION_NAME: A name to use to return serialized data if the data is a list
+ :type COLLECTION_NAME: str
+
+ :param FIELDS_FROM_NONE_TO_EMPTY_STRING: A list of fields to convert to empty string if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str]
+
+ :param FIELDS_FROM_NONE_TO_ZERO: A list of fields to convert to integer zero if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_ZERO: List[str]
+ """
+
+ COLLECTION_NAME: Optional[str] = None
+ FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str] = []
+ FIELDS_FROM_NONE_TO_ZERO: List[str] = []
+
+ def check_collection_name(self):
+ """
+ Method to check that COLLLECTION_NAME attribute is not None
+ """
+ if not self.COLLECTION_NAME:
+ raise AirflowException("The COLLECTION_NAME attribute is missing in the schema")
+
+ @post_dump(pass_many=True)
+ def wrap_with_envelope(self, data, many, **kwargs):
+ """
+ Checks if data is a list and use the envelope key to return it together
+ with total_entries meta
+ :param data: The deserialized data
+ :param many: Whether the data is a collection
+ """
+ self.check_collection_name()
+ if many:
+ data = self._process_list_data(data)
+ return {self.COLLECTION_NAME: data, 'total_entries': len(data)}
Review comment:
You can convert PR to draft
----------------------------------------------------------------
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 #9095: [WIP]Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#issuecomment-642731187
Do you want to add something? I would like to merge this change, but it is a draft, so I can't do 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#issuecomment-642895785
Thanks @ephraimbuddy !
----------------------------------------------------------------
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 #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r435477062
##########
File path: airflow/api_connexion/schemas/base_schema.py
##########
@@ -0,0 +1,79 @@
+# 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.
+from typing import List, Optional
+
+from marshmallow import post_dump
+from marshmallow_sqlalchemy import SQLAlchemySchema
+
+from airflow.exceptions import AirflowException
+
+
+class BaseSchema(SQLAlchemySchema):
+
+ """ Base Schema for sqlalchemy models
+
+ :param COLLECTION_NAME: A name to use to return serialized data if the data is a list
+ :type COLLECTION_NAME: str
+
+ :param FIELDS_FROM_NONE_TO_EMPTY_STRING: A list of fields to convert to empty string if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str]
+
+ :param FIELDS_FROM_NONE_TO_ZERO: A list of fields to convert to integer zero if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_ZERO: List[str]
+ """
+
+ COLLECTION_NAME: Optional[str] = None
+ FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str] = []
+ FIELDS_FROM_NONE_TO_ZERO: List[str] = []
+
+ def check_collection_name(self):
+ """
+ Method to check that COLLLECTION_NAME attribute is not None
+ """
+ if not self.COLLECTION_NAME:
+ raise AirflowException("The COLLECTION_NAME attribute is missing in the schema")
+
+ @post_dump(pass_many=True)
+ def wrap_with_envelope(self, data, many, **kwargs):
+ """
+ Checks if data is a list and use the envelope key to return it together
+ with total_entries meta
+ :param data: The deserialized data
+ :param many: Whether the data is a collection
+ """
+ self.check_collection_name()
+ if many:
+ data = self._process_list_data(data)
+ return {self.COLLECTION_NAME: data, 'total_entries': len(data)}
+ data = self._process_data(data)
+ return data
+
+ def _process_list_data(self, data):
+ return [self._process_data(item) for item in data]
+
+ def _process_data(self, data):
+ d_data = {}
+ for k, v in data.items():
+ if v is None:
+ if k in self.FIELDS_FROM_NONE_TO_EMPTY_STRING:
+ v = ''
+ elif k in self.FIELDS_FROM_NONE_TO_ZERO:
Review comment:
Good. I will try 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on pull request #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#issuecomment-642873825
You can merge it. I don't have anything to add 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] turbaszek commented on a change in pull request #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r434431737
##########
File path: airflow/api_connexion/schemas/connection_schema.py
##########
@@ -0,0 +1,58 @@
+#
+# 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.
+
+from marshmallow_sqlalchemy import auto_field
+
+from airflow.api_connexion.schemas.base_schema import BaseSchema
+from airflow.models.connection import Connection
+
+
+class ConnectionCollectionItemSchema(BaseSchema):
+
Review comment:
```suggestion
```
----------------------------------------------------------------
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 #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r435423087
##########
File path: airflow/api_connexion/schemas/base_schema.py
##########
@@ -0,0 +1,79 @@
+# 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.
+from typing import List, Optional
+
+from marshmallow import post_dump
+from marshmallow_sqlalchemy import SQLAlchemySchema
+
+from airflow.exceptions import AirflowException
+
+
+class BaseSchema(SQLAlchemySchema):
+
+ """ Base Schema for sqlalchemy models
+
+ :param COLLECTION_NAME: A name to use to return serialized data if the data is a list
+ :type COLLECTION_NAME: str
+
+ :param FIELDS_FROM_NONE_TO_EMPTY_STRING: A list of fields to convert to empty string if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str]
+
+ :param FIELDS_FROM_NONE_TO_ZERO: A list of fields to convert to integer zero if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_ZERO: List[str]
+ """
+
+ COLLECTION_NAME: Optional[str] = None
+ FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str] = []
+ FIELDS_FROM_NONE_TO_ZERO: List[str] = []
+
+ def check_collection_name(self):
+ """
+ Method to check that COLLLECTION_NAME attribute is not None
+ """
+ if not self.COLLECTION_NAME:
+ raise AirflowException("The COLLECTION_NAME attribute is missing in the schema")
+
+ @post_dump(pass_many=True)
+ def wrap_with_envelope(self, data, many, **kwargs):
+ """
+ Checks if data is a list and use the envelope key to return it together
+ with total_entries meta
+ :param data: The deserialized data
+ :param many: Whether the data is a collection
+ """
+ self.check_collection_name()
+ if many:
+ data = self._process_list_data(data)
+ return {self.COLLECTION_NAME: data, 'total_entries': len(data)}
Review comment:
Great idea. I wonder if pagination is handled here correctly? When we have 100 entries in the database, the user will fetch only two entries (limit = 2), then total_entires should still be 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] mik-laj commented on pull request #9095: [WIP]Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#issuecomment-640004918
@vbmade2000 Packages should clearly show the content. We already have the api package, so I had to use another package that best describes its content. api_connexion is best in my opinion, because we rely on connexion
----------------------------------------------------------------
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] turbaszek commented on a change in pull request #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r434432445
##########
File path: airflow/api_connexion/schemas/base_schema.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.
+from typing import List, Optional
+
+from marshmallow import post_dump
+from marshmallow_sqlalchemy import SQLAlchemySchema
+
+from airflow.exceptions import AirflowException
+
+
+class BaseSchema(SQLAlchemySchema):
+
+ """ Base Schema for sqlalchemy models
+
+ :param COLLECTION_NAME: A name to use to return serialized data if the data is a list
+ :type COLLECTION_NAME: str
+
+ :param FIELDS_FROM_NONE_TO_EMPTY_STRING: A list of fields to convert to empty string if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str]
+
+ :param FIELDS_FROM_NONE_TO_ZERO: A list of fields to convert to integer zero if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_ZERO: List[str]
+ """
+
+ COLLECTION_NAME: Optional[str] = None
+ FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str] = []
+ FIELDS_FROM_NONE_TO_ZERO: List[str] = []
+
+ def check_collection_name(self):
+ """
+ Method to check that COLLLECTION_NAME attribute is not None
+ """
+ if not self.COLLECTION_NAME:
+ raise AirflowException("The COLLECTION_NAME attribute is missing in the schema")
+
+ @post_dump(pass_many=True)
+ def wrap_with_envelope(self, data, many, **kwargs):
+ """
+ Checks if data is a list and use the envelope key to return it together
+ with total_entries meta
+ :param data: The deserialized data
+ :param many: Whether the data is a collection
+ """
+ self.check_collection_name()
+ if many:
+ data = self._process_list_data(data)
+ return {self.COLLECTION_NAME: data, 'total_entries': len(data)}
+ data = self._process_data(data)
+ return data
+
+ def _process_list_data(self, data):
+ d_data = {}
+ d_list = []
+ for item in data:
+ for k, v in item.items():
+ if v is None and k in self.FIELDS_FROM_NONE_TO_EMPTY_STRING:
+ v = ''
+ elif v is None and k in self.FIELDS_FROM_NONE_TO_ZERO:
+ v = 0
+ d_data[k] = v
+ d_list.append(d_data)
+ d_data = {}
Review comment:
Or even simpler:
```python
def _process_list_data(self, data):
return [self._process_data(item) for item in data]
```
----------------------------------------------------------------
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 #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r434636189
##########
File path: airflow/api_connexion/schemas/base_schema.py
##########
@@ -0,0 +1,78 @@
+# 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.
+from typing import List, Optional
+
+from marshmallow import post_dump
+from marshmallow_sqlalchemy import SQLAlchemySchema
+
+from airflow.exceptions import AirflowException
+
+
+class BaseSchema(SQLAlchemySchema):
+
+ """ Base Schema for sqlalchemy models
+
+ :param COLLECTION_NAME: A name to use to return serialized data if the data is a list
+ :type COLLECTION_NAME: str
+
+ :param FIELDS_FROM_NONE_TO_EMPTY_STRING: A list of fields to convert to empty string if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str]
+
+ :param FIELDS_FROM_NONE_TO_ZERO: A list of fields to convert to integer zero if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_ZERO: List[str]
+ """
+
+ COLLECTION_NAME: Optional[str] = None
+ FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str] = []
+ FIELDS_FROM_NONE_TO_ZERO: List[str] = []
+
+ def check_collection_name(self):
+ """
+ Method to check that COLLLECTION_NAME attribute is not None
+ """
+ if not self.COLLECTION_NAME:
+ raise AirflowException("The COLLECTION_NAME attribute is missing in the schema")
+
+ @post_dump(pass_many=True)
+ def wrap_with_envelope(self, data, many, **kwargs):
+ """
+ Checks if data is a list and use the envelope key to return it together
+ with total_entries meta
+ :param data: The deserialized data
+ :param many: Whether the data is a collection
+ """
+ self.check_collection_name()
+ if many:
+ data = self._process_list_data(data)
+ return {self.COLLECTION_NAME: data, 'total_entries': len(data)}
+ data = self._process_data(data)
+ return data
+
+ def _process_list_data(self, data):
+ return [self._process_data(item) for item in data]
+
+ def _process_data(self, data):
+ d_data = {}
+ for k, v in data.items():
+ if v is None and k in self.FIELDS_FROM_NONE_TO_EMPTY_STRING:
+ v = ''
+ elif v is None and k in self.FIELDS_FROM_NONE_TO_ZERO:
+ v = 0
+ d_data[k] = v
+ return d_data
Review comment:
I tried having it exactly like above but had issues with my tests. I have updated it similar to this
----------------------------------------------------------------
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 #9095: [WIP]Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r436117593
##########
File path: airflow/api_connexion/schemas/base_schema.py
##########
@@ -0,0 +1,79 @@
+# 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.
+from typing import List, Optional
+
+from marshmallow import post_dump
+from marshmallow_sqlalchemy import SQLAlchemySchema
+
+from airflow.exceptions import AirflowException
+
+
+class BaseSchema(SQLAlchemySchema):
+
+ """ Base Schema for sqlalchemy models
+
+ :param COLLECTION_NAME: A name to use to return serialized data if the data is a list
+ :type COLLECTION_NAME: str
+
+ :param FIELDS_FROM_NONE_TO_EMPTY_STRING: A list of fields to convert to empty string if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str]
+
+ :param FIELDS_FROM_NONE_TO_ZERO: A list of fields to convert to integer zero if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_ZERO: List[str]
+ """
+
+ COLLECTION_NAME: Optional[str] = None
+ FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str] = []
+ FIELDS_FROM_NONE_TO_ZERO: List[str] = []
+
+ def check_collection_name(self):
+ """
+ Method to check that COLLLECTION_NAME attribute is not None
+ """
+ if not self.COLLECTION_NAME:
+ raise AirflowException("The COLLECTION_NAME attribute is missing in the schema")
+
+ @post_dump(pass_many=True)
+ def wrap_with_envelope(self, data, many, **kwargs):
+ """
+ Checks if data is a list and use the envelope key to return it together
+ with total_entries meta
+ :param data: The deserialized data
+ :param many: Whether the data is a collection
+ """
+ self.check_collection_name()
+ if many:
+ data = self._process_list_data(data)
+ return {self.COLLECTION_NAME: data, 'total_entries': len(data)}
+ data = self._process_data(data)
+ return data
+
+ def _process_list_data(self, data):
+ return [self._process_data(item) for item in data]
+
+ def _process_data(self, data):
+ d_data = {}
+ for k, v in data.items():
+ if v is None:
+ if k in self.FIELDS_FROM_NONE_TO_EMPTY_STRING:
+ v = ''
+ elif k in self.FIELDS_FROM_NONE_TO_ZERO:
Review comment:
Hi, I was not able to fix this by extending fields but adding nullable to the specification like you said fixed it. This is the fix [819e036](https://github.com/apache/airflow/pull/9095/commits/819e036694f92e9a805941e4d464884853f24791)
Also I am thinking that the spec changes can be done in a separate 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 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 #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r435425637
##########
File path: airflow/api_connexion/schemas/base_schema.py
##########
@@ -0,0 +1,79 @@
+# 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.
+from typing import List, Optional
+
+from marshmallow import post_dump
+from marshmallow_sqlalchemy import SQLAlchemySchema
+
+from airflow.exceptions import AirflowException
+
+
+class BaseSchema(SQLAlchemySchema):
+
+ """ Base Schema for sqlalchemy models
+
+ :param COLLECTION_NAME: A name to use to return serialized data if the data is a list
+ :type COLLECTION_NAME: str
+
+ :param FIELDS_FROM_NONE_TO_EMPTY_STRING: A list of fields to convert to empty string if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str]
+
+ :param FIELDS_FROM_NONE_TO_ZERO: A list of fields to convert to integer zero if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_ZERO: List[str]
+ """
+
+ COLLECTION_NAME: Optional[str] = None
+ FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str] = []
+ FIELDS_FROM_NONE_TO_ZERO: List[str] = []
+
+ def check_collection_name(self):
+ """
+ Method to check that COLLLECTION_NAME attribute is not None
+ """
+ if not self.COLLECTION_NAME:
+ raise AirflowException("The COLLECTION_NAME attribute is missing in the schema")
+
+ @post_dump(pass_many=True)
+ def wrap_with_envelope(self, data, many, **kwargs):
+ """
+ Checks if data is a list and use the envelope key to return it together
+ with total_entries meta
+ :param data: The deserialized data
+ :param many: Whether the data is a collection
+ """
+ self.check_collection_name()
+ if many:
+ data = self._process_list_data(data)
+ return {self.COLLECTION_NAME: data, 'total_entries': len(data)}
+ data = self._process_data(data)
+ return data
+
+ def _process_list_data(self, data):
+ return [self._process_data(item) for item in data]
+
+ def _process_data(self, data):
+ d_data = {}
+ for k, v in data.items():
+ if v is None:
+ if k in self.FIELDS_FROM_NONE_TO_EMPTY_STRING:
+ v = ''
+ elif k in self.FIELDS_FROM_NONE_TO_ZERO:
Review comment:
It seems to me that we could do this by extending the fields.String or fields.Int class. It will be clearer.
----------------------------------------------------------------
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 #9095: [WIP]Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r437016593
##########
File path: airflow/api_connexion/schemas/base_schema.py
##########
@@ -0,0 +1,79 @@
+# 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.
+from typing import List, Optional
+
+from marshmallow import post_dump
+from marshmallow_sqlalchemy import SQLAlchemySchema
+
+from airflow.exceptions import AirflowException
+
+
+class BaseSchema(SQLAlchemySchema):
+
+ """ Base Schema for sqlalchemy models
+
+ :param COLLECTION_NAME: A name to use to return serialized data if the data is a list
+ :type COLLECTION_NAME: str
+
+ :param FIELDS_FROM_NONE_TO_EMPTY_STRING: A list of fields to convert to empty string if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str]
+
+ :param FIELDS_FROM_NONE_TO_ZERO: A list of fields to convert to integer zero if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_ZERO: List[str]
+ """
+
+ COLLECTION_NAME: Optional[str] = None
+ FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str] = []
+ FIELDS_FROM_NONE_TO_ZERO: List[str] = []
+
+ def check_collection_name(self):
+ """
+ Method to check that COLLLECTION_NAME attribute is not None
+ """
+ if not self.COLLECTION_NAME:
+ raise AirflowException("The COLLECTION_NAME attribute is missing in the schema")
+
+ @post_dump(pass_many=True)
+ def wrap_with_envelope(self, data, many, **kwargs):
+ """
+ Checks if data is a list and use the envelope key to return it together
+ with total_entries meta
+ :param data: The deserialized data
+ :param many: Whether the data is a collection
+ """
+ self.check_collection_name()
+ if many:
+ data = self._process_list_data(data)
+ return {self.COLLECTION_NAME: data, 'total_entries': len(data)}
+ data = self._process_data(data)
+ return data
+
+ def _process_list_data(self, data):
+ return [self._process_data(item) for item in data]
+
+ def _process_data(self, data):
+ d_data = {}
+ for k, v in data.items():
+ if v is None:
+ if k in self.FIELDS_FROM_NONE_TO_EMPTY_STRING:
+ v = ''
+ elif k in self.FIELDS_FROM_NONE_TO_ZERO:
Review comment:
> Also I am thinking that the spec changes can be done in a separate PR
You can suggest it. For me, it's okay if the changes are proposed with context.
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1146,14 +1146,19 @@ components:
type: string
conn_type:
type: string
+ nullable: true
Review comment:
This fielld should be non-nullable. I created separate PR about the database schema.
https://github.com/apache/airflow/pull/9187
----------------------------------------------------------------
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 #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r435426778
##########
File path: tests/api_connexion/endpoints/test_connection_endpoint.py
##########
@@ -29,38 +30,99 @@ def setUpClass(cls) -> None:
def setUp(self) -> None:
self.client = self.app.test_client() # type:ignore
+ # we want only the connection created here for this test
+ with create_session() as session:
+ session.query(Connection).delete()
+
+ def tearDown(self) -> None:
+ clear_db_connections()
class TestDeleteConnection(TestConnectionEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
+ @unittest.skip("Not implemented yet")
def test_should_response_200(self):
response = self.client.delete("/api/v1/connections/1")
assert response.status_code == 200
class TestGetConnection(TestConnectionEndpoint):
- @pytest.mark.skip(reason="Not implemented yet")
- def test_should_response_200(self):
- response = self.client.get("/api/v1/connection/1")
+
+ @provide_session
+ def test_should_response_200(self, session):
Review comment:
Can you add support for 404 as well?
----------------------------------------------------------------
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] vbmade2000 commented on pull request #9095: [WIP]Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
vbmade2000 commented on pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#issuecomment-640001400
A question. Should we have a directory/filename with package name in it ? Like here we have airflow/api_connexion/endpoints/connection_endpoint.py. This path contains connexion name in it. IMHO code should not reflect underlying package. Just my opinion.
----------------------------------------------------------------
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 #9095: [WIP]Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r437559058
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1146,14 +1146,19 @@ components:
type: string
conn_type:
type: string
+ nullable: true
Review comment:
Ok. Can you do rebase? My change has been merged.
----------------------------------------------------------------
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] turbaszek merged pull request #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
turbaszek merged pull request #9095:
URL: https://github.com/apache/airflow/pull/9095
----------------------------------------------------------------
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 #9095: [WIP]Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r438023424
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1148,12 +1148,16 @@ components:
type: string
host:
type: string
+ nullable: true
Review comment:
+1
----------------------------------------------------------------
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 #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r435424588
##########
File path: airflow/api_connexion/schemas/base_schema.py
##########
@@ -0,0 +1,79 @@
+# 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.
+from typing import List, Optional
+
+from marshmallow import post_dump
+from marshmallow_sqlalchemy import SQLAlchemySchema
+
+from airflow.exceptions import AirflowException
+
+
+class BaseSchema(SQLAlchemySchema):
+
+ """ Base Schema for sqlalchemy models
+
+ :param COLLECTION_NAME: A name to use to return serialized data if the data is a list
+ :type COLLECTION_NAME: str
+
+ :param FIELDS_FROM_NONE_TO_EMPTY_STRING: A list of fields to convert to empty string if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str]
+
+ :param FIELDS_FROM_NONE_TO_ZERO: A list of fields to convert to integer zero if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_ZERO: List[str]
+ """
+
+ COLLECTION_NAME: Optional[str] = None
+ FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str] = []
+ FIELDS_FROM_NONE_TO_ZERO: List[str] = []
+
+ def check_collection_name(self):
+ """
+ Method to check that COLLLECTION_NAME attribute is not None
+ """
+ if not self.COLLECTION_NAME:
+ raise AirflowException("The COLLECTION_NAME attribute is missing in the schema")
+
+ @post_dump(pass_many=True)
+ def wrap_with_envelope(self, data, many, **kwargs):
+ """
+ Checks if data is a list and use the envelope key to return it together
+ with total_entries meta
+ :param data: The deserialized data
+ :param many: Whether the data is a collection
+ """
+ self.check_collection_name()
+ if many:
+ data = self._process_list_data(data)
+ return {self.COLLECTION_NAME: data, 'total_entries': len(data)}
+ data = self._process_data(data)
+ return data
+
+ def _process_list_data(self, data):
+ return [self._process_data(item) for item in data]
+
+ def _process_data(self, data):
+ d_data = {}
+ for k, v in data.items():
+ if v is None:
+ if k in self.FIELDS_FROM_NONE_TO_EMPTY_STRING:
Review comment:
I think we should add nullable to the specification in a few places. This was not done early because we focused on the general API specification.
----------------------------------------------------------------
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 #9095: [WIP]Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r437590382
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1146,14 +1146,19 @@ components:
type: string
conn_type:
type: string
+ nullable: true
Review comment:
I hurriedly pushed now. I will still come back to it this night. [e40d532](https://github.com/apache/airflow/pull/9095/commits/e40d532c01461672514fa07291633bb3796fd9b6)
----------------------------------------------------------------
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 #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r435500200
##########
File path: airflow/api_connexion/schemas/base_schema.py
##########
@@ -0,0 +1,79 @@
+# 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.
+from typing import List, Optional
+
+from marshmallow import post_dump
+from marshmallow_sqlalchemy import SQLAlchemySchema
+
+from airflow.exceptions import AirflowException
+
+
+class BaseSchema(SQLAlchemySchema):
+
+ """ Base Schema for sqlalchemy models
+
+ :param COLLECTION_NAME: A name to use to return serialized data if the data is a list
+ :type COLLECTION_NAME: str
+
+ :param FIELDS_FROM_NONE_TO_EMPTY_STRING: A list of fields to convert to empty string if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str]
+
+ :param FIELDS_FROM_NONE_TO_ZERO: A list of fields to convert to integer zero if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_ZERO: List[str]
+ """
+
+ COLLECTION_NAME: Optional[str] = None
+ FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str] = []
+ FIELDS_FROM_NONE_TO_ZERO: List[str] = []
+
+ def check_collection_name(self):
+ """
+ Method to check that COLLLECTION_NAME attribute is not None
+ """
+ if not self.COLLECTION_NAME:
+ raise AirflowException("The COLLECTION_NAME attribute is missing in the schema")
+
+ @post_dump(pass_many=True)
+ def wrap_with_envelope(self, data, many, **kwargs):
+ """
+ Checks if data is a list and use the envelope key to return it together
+ with total_entries meta
+ :param data: The deserialized data
+ :param many: Whether the data is a collection
+ """
+ self.check_collection_name()
+ if many:
+ data = self._process_list_data(data)
+ return {self.COLLECTION_NAME: data, 'total_entries': len(data)}
Review comment:
It works on pagination. I will push now. Please, Is there a way I can change this PR back to draft?
----------------------------------------------------------------
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 #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r435478319
##########
File path: airflow/api_connexion/schemas/base_schema.py
##########
@@ -0,0 +1,79 @@
+# 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.
+from typing import List, Optional
+
+from marshmallow import post_dump
+from marshmallow_sqlalchemy import SQLAlchemySchema
+
+from airflow.exceptions import AirflowException
+
+
+class BaseSchema(SQLAlchemySchema):
+
+ """ Base Schema for sqlalchemy models
+
+ :param COLLECTION_NAME: A name to use to return serialized data if the data is a list
+ :type COLLECTION_NAME: str
+
+ :param FIELDS_FROM_NONE_TO_EMPTY_STRING: A list of fields to convert to empty string if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str]
+
+ :param FIELDS_FROM_NONE_TO_ZERO: A list of fields to convert to integer zero if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_ZERO: List[str]
+ """
+
+ COLLECTION_NAME: Optional[str] = None
+ FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str] = []
+ FIELDS_FROM_NONE_TO_ZERO: List[str] = []
+
+ def check_collection_name(self):
+ """
+ Method to check that COLLLECTION_NAME attribute is not None
+ """
+ if not self.COLLECTION_NAME:
+ raise AirflowException("The COLLECTION_NAME attribute is missing in the schema")
+
+ @post_dump(pass_many=True)
+ def wrap_with_envelope(self, data, many, **kwargs):
+ """
+ Checks if data is a list and use the envelope key to return it together
+ with total_entries meta
+ :param data: The deserialized data
+ :param many: Whether the data is a collection
+ """
+ self.check_collection_name()
+ if many:
+ data = self._process_list_data(data)
+ return {self.COLLECTION_NAME: data, 'total_entries': len(data)}
+ data = self._process_data(data)
+ return data
+
+ def _process_list_data(self, data):
+ return [self._process_data(item) for item in data]
+
+ def _process_data(self, data):
+ d_data = {}
+ for k, v in data.items():
+ if v is None:
+ if k in self.FIELDS_FROM_NONE_TO_EMPTY_STRING:
Review comment:
That would be great,
----------------------------------------------------------------
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] turbaszek commented on a change in pull request #9095: Add readonly connection endpoints
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #9095:
URL: https://github.com/apache/airflow/pull/9095#discussion_r434523838
##########
File path: airflow/api_connexion/schemas/base_schema.py
##########
@@ -0,0 +1,78 @@
+# 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.
+from typing import List, Optional
+
+from marshmallow import post_dump
+from marshmallow_sqlalchemy import SQLAlchemySchema
+
+from airflow.exceptions import AirflowException
+
+
+class BaseSchema(SQLAlchemySchema):
+
+ """ Base Schema for sqlalchemy models
+
+ :param COLLECTION_NAME: A name to use to return serialized data if the data is a list
+ :type COLLECTION_NAME: str
+
+ :param FIELDS_FROM_NONE_TO_EMPTY_STRING: A list of fields to convert to empty string if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str]
+
+ :param FIELDS_FROM_NONE_TO_ZERO: A list of fields to convert to integer zero if value is None
+ after serialization
+ :type FIELDS_FROM_NONE_TO_ZERO: List[str]
+ """
+
+ COLLECTION_NAME: Optional[str] = None
+ FIELDS_FROM_NONE_TO_EMPTY_STRING: List[str] = []
+ FIELDS_FROM_NONE_TO_ZERO: List[str] = []
+
+ def check_collection_name(self):
+ """
+ Method to check that COLLLECTION_NAME attribute is not None
+ """
+ if not self.COLLECTION_NAME:
+ raise AirflowException("The COLLECTION_NAME attribute is missing in the schema")
+
+ @post_dump(pass_many=True)
+ def wrap_with_envelope(self, data, many, **kwargs):
+ """
+ Checks if data is a list and use the envelope key to return it together
+ with total_entries meta
+ :param data: The deserialized data
+ :param many: Whether the data is a collection
+ """
+ self.check_collection_name()
+ if many:
+ data = self._process_list_data(data)
+ return {self.COLLECTION_NAME: data, 'total_entries': len(data)}
+ data = self._process_data(data)
+ return data
+
+ def _process_list_data(self, data):
+ return [self._process_data(item) for item in data]
+
+ def _process_data(self, data):
+ d_data = {}
+ for k, v in data.items():
+ if v is None and k in self.FIELDS_FROM_NONE_TO_EMPTY_STRING:
+ v = ''
+ elif v is None and k in self.FIELDS_FROM_NONE_TO_ZERO:
+ v = 0
+ d_data[k] = v
+ return d_data
Review comment:
```suggestion
d_data = {}
for k, v in data.items():
if v is not None:
continue
elif k in self.FIELDS_FROM_NONE_TO_EMPTY_STRING:
v = ''
elif k in self.FIELDS_FROM_NONE_TO_ZERO:
v = 0
d_data[k] = v
return d_data
```
WDYT?
----------------------------------------------------------------
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