You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "potiuk (via GitHub)" <gi...@apache.org> on 2023/03/12 14:05:07 UTC

[GitHub] [airflow] potiuk commented on pull request #28970: Fix support for macros with dots in DataProcJobBuilder

potiuk commented on PR #28970:
URL: https://github.com/apache/airflow/pull/28970#issuecomment-1465208633

   I think this is really nice feature to have to use the DataProcJobBuillder instead of "raw" dictionary and it was - I believe the original intention of the builder - and while it is used like that internally by the "specific" operators, there  should generally be no problem with it. So I would see that as a nice new "feature" to add. Yes it worked (to some extend before sanitization - but that was more of an accident than intention and sanitization indeed broke it.
   
   Why don't we turn it into a "real" feature:
   
   * explain that it can be done
   * implement a way to sanitize the job name in a different place. 
   
   I believe (correct me if I am wrong) , 
   
   
   ```
   ```
   
   Is there any reason we cannot move the sanitization to inside the `execute` method ? Would that break anything other than delaying sanitization to the moment of execution? I guess this is usually what we do with potentially-template'able fields, the benefits of them being templated far outweight the "early" sanitization during parsing, and we have generally pushed such code to execute() method in multiple places. 


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