You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/03/03 20:05:49 UTC

[GitHub] [airflow] jmcarp opened a new pull request #14589: Drop third-party lazy proxy.

jmcarp opened a new pull request #14589:
URL: https://github.com/apache/airflow/pull/14589


   We already get a local object proxy from werkzeug, and it's trivial to
   add memoization with the standard library, so we can drop the
   third-party dependency without losing functionality. This patch also
   adds explicit getters to the template context for fields that require a
   database lookup for explicitness.


----------------------------------------------------------------
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] jmcarp commented on a change in pull request #14589: Drop third-party lazy proxy.

Posted by GitBox <gi...@apache.org>.
jmcarp commented on a change in pull request #14589:
URL: https://github.com/apache/airflow/pull/14589#discussion_r588834321



##########
File path: airflow/utils/proxy.py
##########
@@ -0,0 +1,47 @@
+#
+# 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 functools
+from typing import Any, Callable
+
+from werkzeug.local import LocalProxy
+
+
+class _Miss:
+    pass
+
+
+_miss = _Miss()
+
+
+def cached_getter(getter: Callable[[], Any]) -> Callable[[], Any]:
+    """Return cached version of getter function."""
+    cached = _miss
+    @functools.wraps(getter)
+    def wrapped():
+        nonlocal cached
+        if cached is _miss:
+            cached = getter()
+        return cached
+    return wrapped

Review comment:
       This was the first thing I tried, but I ran into serialization issues with the virtualenvoperator. Writing my own caching decorator made the return value of `cached_getter` serializable, but now I'm stuck on getting werkzeug `LocalProxy` to serialize correctly with that operator. I haven't dug into it too far, but maybe `lazy-object-proxy` does some magic to make de/serializing lambdas work. I'm not sure this is worth pursuing unless you have any ideas about fixing serialization.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #14589: Drop third-party lazy proxy.

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #14589:
URL: https://github.com/apache/airflow/pull/14589#discussion_r588204498



##########
File path: airflow/utils/proxy.py
##########
@@ -0,0 +1,47 @@
+#
+# 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 functools
+from typing import Any, Callable
+
+from werkzeug.local import LocalProxy
+
+
+class _Miss:
+    pass
+
+
+_miss = _Miss()
+
+
+def cached_getter(getter: Callable[[], Any]) -> Callable[[], Any]:
+    """Return cached version of getter function."""
+    cached = _miss
+    @functools.wraps(getter)
+    def wrapped():
+        nonlocal cached
+        if cached is _miss:
+            cached = getter()
+        return cached
+    return wrapped

Review comment:
       ```suggestion
   
   _T = TypeVar("T")
   
   def cached_getter(getter: Callable[[], T]) -> Callable[[], T]:
       """Return cached version of getter function."""
       return functools.lru_cache(maxsize=None)(getter)
   ```




----------------------------------------------------------------
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] jmcarp closed pull request #14589: Drop third-party lazy proxy.

Posted by GitBox <gi...@apache.org>.
jmcarp closed pull request #14589:
URL: https://github.com/apache/airflow/pull/14589


   


----------------------------------------------------------------
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