[GitHub] [airflow] potiuk edited a comment on pull request #20540: Remove `host` as an instance attr in `DatabricksHook`

[GitBox <gi...@apache.org>]

potiuk edited a comment on pull request #20540:
URL: https://github.com/apache/airflow/pull/20540#issuecomment-1002263115


   I think "behavrioural" changes like that are not "breaking". Whether `self.host` is an instance of variable does not change the "use" of it - at most it optimizes some behaviours - in this case the "self.host" was created in both - DagParser and Task, when it was removed, it was only created in Task (during execute).
   
   As we discussed here: https://github.com/apache/airflow/pull/19572#discussion_r776052141 - Databricks is not really a "generic" operator that you might want to extend or dertive from to use internal features. Unlke KPO for example which is generic and it's easier to imagine that people would like to change their behaviour by extending it. So I think we can consider it's internals are really "implementation details".
   
   Unfortunately we do not have clear-cut what is breaking and what not :(. Theoretically even adding a default parameter might be breaking because somone could overload that method in a derived class, or you could imagine someone checks signature of the method.
   
   This is really almost always an exercise of "how likely this change is breaking" rather than "clear-cut" 0/1 breaking/non-breaking. And except some obvious cases, almost always it will have to be judged individually.
   


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