potiuk commented on pull request #19725: URL: https://github.com/apache/airflow/pull/19725#issuecomment-975990492
Yeah feel free to raise it on the list :). I think we have no "hard" rules defined for the providers - except the SamVer which tell that you MAY introduce breaaking change (i.e. drop parameters) when you release major upgrade. So technically, we could drop those deprecated parameters with any MAJOR release of provider. However I am of a position that not everything that can be done - should be. I think keeping backwards compatibility is extremely important - and except stuff that we HAVE have as backwards incompatibility, I think we should drop something only when we have very good reason - i.e. when it slows us down on releasing something or when it costs a lot to maintain it. In vast majority of cases I see no reason for dropping a parameter if it is just renaming and keeping old one for backwards compatibility, This has very little benefit (two lines of code less) but can potentially wreak havoc for an unsuspecting user. So my approach is: We should drop any such deprecated parameters when we "have to" (because we have good reason) not when we "can" However this is a different case @dimon222 talks about (please confirm @dimon222) From what @dimon222 understand says is that deprecation warnings are raised accidentaly whenever there is a legitimate use of the Operator and it cannot be avoided. Usually when you have such depracation there should be an easy way to change your use of it and deprecation warning should disappear. I do not know that much about the the operator and it's parameter to judge it - but I find the `if iam` logic a bit suspicious and some questions should be answered: 1) should the client_type be deprecated (as it was in #17987) - seems that 'if client_type == 'iam' suggests that there is still legitimate use of it ? Why? 2) is there a way to skip client_type and do what @dimon222 wants to do without raising deprecation warnining 3) finally is the deprecation warning clear-enough and explains the user how to convert their code to avoid the deprecation warning? As I wrote - I do not know much about the reasons for deprecating client_type - but if we answer the three questions above, we should be able to decide if deprecation should really be there. Also @eladkal maybe you can chime in here? -- 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]
