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/07/05 14:25:41 UTC

[GitHub] [airflow] codenamestif opened a new pull request #16820: Fix wrong template_fields_renderers for AWS operators

codenamestif opened a new pull request #16820:
URL: https://github.com/apache/airflow/pull/16820


   Related to #16808
   
   Fixing wrong `template_fields_renderers` for Sagemaker and ECS operators. There might be other affected operators, but i'm fixing the ones i can test.


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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] kaxil commented on a change in pull request #16820: Fix wrong template_fields_renderers for AWS operators

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



##########
File path: airflow/providers/amazon/aws/operators/ecs.py
##########
@@ -144,7 +144,7 @@ class ECSOperator(BaseOperator):
 
     ui_color = '#f0ede4'
     template_fields = ('overrides',)
-    template_fields_renderers = {"overrides": "py"}
+    template_fields_renderers = {"overrides": "json"}

Review comment:
       Awesome, thanks. Sounds 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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] kaxil commented on a change in pull request #16820: Fix wrong template_fields_renderers for AWS operators

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



##########
File path: airflow/providers/amazon/aws/operators/ecs.py
##########
@@ -144,7 +144,7 @@ class ECSOperator(BaseOperator):
 
     ui_color = '#f0ede4'
     template_fields = ('overrides',)
-    template_fields_renderers = {"overrides": "py"}
+    template_fields_renderers = {"overrides": "json"}

Review comment:
       Overrides is a dict no a json , no ?




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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] codenamestif commented on a change in pull request #16820: Fix wrong template_fields_renderers for AWS operators

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



##########
File path: airflow/providers/amazon/aws/operators/ecs.py
##########
@@ -144,7 +144,7 @@ class ECSOperator(BaseOperator):
 
     ui_color = '#f0ede4'
     template_fields = ('overrides',)
-    template_fields_renderers = {"overrides": "py"}
+    template_fields_renderers = {"overrides": "json"}

Review comment:
       @kaxil yes, it's a dict, but then `py` renderer says that there is no source code for dict class. So i thought `json` fix 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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] kaxil commented on a change in pull request #16820: Fix wrong template_fields_renderers for AWS operators

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



##########
File path: airflow/providers/amazon/aws/operators/ecs.py
##########
@@ -144,7 +144,7 @@ class ECSOperator(BaseOperator):
 
     ui_color = '#f0ede4'
     template_fields = ('overrides',)
-    template_fields_renderers = {"overrides": "py"}
+    template_fields_renderers = {"overrides": "json"}

Review comment:
       Can you post a before/after screenshot for the view in Webserver, please?




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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] potiuk merged pull request #16820: Fix wrong template_fields_renderers for AWS operators

Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #16820:
URL: https://github.com/apache/airflow/pull/16820


   


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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] kaxil commented on a change in pull request #16820: Fix wrong template_fields_renderers for AWS operators

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



##########
File path: airflow/providers/amazon/aws/operators/ecs.py
##########
@@ -144,7 +144,7 @@ class ECSOperator(BaseOperator):
 
     ui_color = '#f0ede4'
     template_fields = ('overrides',)
-    template_fields_renderers = {"overrides": "py"}
+    template_fields_renderers = {"overrides": "json"}

Review comment:
       Can you post a before/after screenshot, please?




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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] codenamestif commented on pull request #16820: Fix wrong template_fields_renderers for AWS operators

Posted by GitBox <gi...@apache.org>.
codenamestif commented on pull request #16820:
URL: https://github.com/apache/airflow/pull/16820#issuecomment-874558178


   @uranusjr could you please have a look? Thanks.


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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] codenamestif commented on a change in pull request #16820: Fix wrong template_fields_renderers for AWS operators

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



##########
File path: airflow/providers/amazon/aws/operators/ecs.py
##########
@@ -144,7 +144,7 @@ class ECSOperator(BaseOperator):
 
     ui_color = '#f0ede4'
     template_fields = ('overrides',)
-    template_fields_renderers = {"overrides": "py"}
+    template_fields_renderers = {"overrides": "json"}

Review comment:
       It looks really good. Yes, it might be a bit confusing since it's not really json, but it formats it correctly.
   
   ![image](https://user-images.githubusercontent.com/2885779/125200541-ee296380-e26b-11eb-8508-6912a29142de.png)
   
   You can check the before screenshot in the issue.
   




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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] codenamestif commented on pull request #16820: Fix wrong template_fields_renderers for AWS operators

Posted by GitBox <gi...@apache.org>.
codenamestif commented on pull request #16820:
URL: https://github.com/apache/airflow/pull/16820#issuecomment-874558178


   @uranusjr could you please have a look? Thanks.


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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] codenamestif commented on a change in pull request #16820: Fix wrong template_fields_renderers for AWS operators

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



##########
File path: airflow/providers/amazon/aws/operators/ecs.py
##########
@@ -144,7 +144,7 @@ class ECSOperator(BaseOperator):
 
     ui_color = '#f0ede4'
     template_fields = ('overrides',)
-    template_fields_renderers = {"overrides": "py"}
+    template_fields_renderers = {"overrides": "json"}

Review comment:
       @kaxil yes, it's a dict, but then `py` renderer says that there is no source code for dict class. So i thought `json` fixes 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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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