dpgaspar commented on a change in pull request #11704:
URL: 
https://github.com/apache/incubator-superset/pull/11704#discussion_r525036264



##########
File path: superset/jinja_context.py
##########
@@ -266,6 +338,15 @@ class PrestoTemplateProcessor(BaseTemplateProcessor):
 
     engine = "presto"
 
+    def set_context(self, **kwargs: Any) -> None:
+        super().set_context(**kwargs)
+        self._context[self.engine] = {

Review comment:
       A bit confused here on the parent we have `self._context.update({ ... 
})` and on Presto we set it inside the engine key, shouldn't they all live on 
at the dict root level?

##########
File path: UPDATING.md
##########
@@ -38,7 +40,11 @@ assists people when migrating to a new version.
   and requires more work. You can easily turn on the languages you want
   to expose in your environment in superset_config.py
 
+<<<<<<< HEAD
+* [11172](https://github.com/apache/incubator-superset/pull/11172): Breaking 
change: SQL templating is turned off be default. To turn it on set 
`ENABLE_TEMPLATE_PROCESSING` to True in `FEATURE_FLAGS`
+=======
 - [11172](https://github.com/apache/incubator-superset/pull/11172): Breaking 
change: SQL templating is turned off be default. To turn it on set 
`ENABLE_TEMPLATE_PROCESSING` to True on `DEFAULT_FEATURE_FLAGS`
+>>>>>>> master

Review comment:
       This seems like a merge conflict, while your at it can you fix `turned 
off be default` to `turned off by default`

##########
File path: superset/jinja_context.py
##########
@@ -266,6 +338,15 @@ class PrestoTemplateProcessor(BaseTemplateProcessor):
 
     engine = "presto"
 
+    def set_context(self, **kwargs: Any) -> None:
+        super().set_context(**kwargs)
+        self._context[self.engine] = {
+            "first_latest_partition": partial(safe_proxy, 
self.first_latest_partition),
+            "latest_partitions": partial(safe_proxy, self.latest_partitions),
+            "latest_sub_partition": partial(safe_proxy, 
self.latest_sub_partition),

Review comment:
       nit: Not totally related with this PR but it's strange that `def 
latest_sub_partition(self, table_name: str, **kwargs: Any) -> Any:` returns 
`Any` should probably be `Optional[str]`




----------------------------------------------------------------
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to