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

   Yeah. i think we should add it. 
   
   Context matters. A lot. I think we cannot say 'renaming protected class is 
always breaking' or renaming protected class is always not breaking'. It very 
much depends on what we are renaming.
   
   As i explained several times before - my view on deciding whether a change 
is breaking or not is never 0-1. It should be based on our assessment on how 
likely it is to break heavily the workflows of a number of our users.
   
   @ashb posted it several times already but this explains it very well:
   
   
![20220730_193042.jpg](https://user-images.githubusercontent.com/595491/185733523-d5187562-b12e-4604-92e9-3be3aa90fe5e.jpg)
   
   Python is really a dynamic language and we do not have fixed 'APIs' that 
would let us always know with 100% certainty that we introduce a breaking 
change. We have a bit of dualism (similarly as in case of dependencies) that 
airflow is both an application (when it comes to airflow core) and library 
intended to be used by our users (in case of DAG authoring). And breaking/not 
breaking has different meaning in those two cases.
   
   There really no true 'private'  methods in Python, the underscores (even the 
double ones) are really convention and expressions of intention. In this case 
since this is a public base Hook class and the intention is for users to extend 
the base class, the single underscore is an intention to use it in classes that 
users create. And my assessment in this case is that the likelihood of breaking 
user's workflows is high. Hooks are definitely parts of our 'public' APIs and 
in this case protected classes are intended for our users to use.
   
   But In case of classes that are not intended to be extended and are purely 
airflow internal,  and not part of the real user APIs, this is a bit different 
story. There, protected methods are for ourselves - maintainers and developers 
of Airflow, not for our users. And intention is that if the class is later 
extended (internally in Airflek core only) those classes should be able to use 
it. And we have full control where the method is used because it should only be 
done in Airflow and never in user's code.
   
   In such case, it is less likely that someone bases their workflow on those, 
and even if they do, they are clearly not doing what was intended. And in this 
case i'd say renaming a  protected class is not a breaking changes.
   
   Sorry for such long comment, but I think it is really important to 
understand that breaking/not breaking is not as clear 0-1 decision in case of 
Airflow as you might think.
   
   BTW. When we split providers to separate repos i have a dream (I am thinking 
hard of various cases it involves) that we will be able to make much clearer 
distinction and introduce much more explicit and clear APIs that will separate 
Airflow developers from Airflow users and make such decisions breaking/not 
breaking much easier. This is - IMHO a bit of necessity to keep the providers 
in check when they are separated, and i hope we will be able to pull that one 
off.


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

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to