potiuk commented on PR #30727: URL: https://github.com/apache/airflow/pull/30727#issuecomment-1534517947
Yeah - if we do NOT merge this one to 2.6.1, this is going to cause us a major pain (Sorry @o-nikolas for inflicting that pain on you again, but I merged the #31033 before that one to show exactly the kinf of pain we might have if we merge this one but do not cherry-pick it to 2.6 branch: see my comment here https://github.com/apache/airflow/pull/31033#issuecomment-1534466680 > My concern in this PR is on the release side. Do folks agree to class this as a miscellaneous change in 2.6.1 because without including it in 2.6.1 we will have conflicts if there are bug fixes from previous release? Yes, if we do not merge that one, we will have problems of the same class that @o-nikolas now has - only those will be problems of release managers for 2.6.* bugfix releases, not the author of the change. Release manager's work should be mainly mechanical and if it involves risky conflict solving and has the potential of introducing bugs while cherry-picking, this is is for me a very bad idea. > This is not a new feature. It's optimization improvement. I don't see any issue with releasing in it in 2.6.1 I would only be for merging (and then immediate cherry-picking that one for the reson described above) after we are absolutely sure it does not introduce any bugs or causes performance regressions. This is one of the things why "reviewability" of such change is such important - if there is a thing that has the potential to break many people's workfllows, we should be extra careful especially if it is going to be released in any of the patchlevels We keep on telling people, that the patchlevel releases are only "fixing" stuff - and they are "super-safe" to migrate to the latest patchlevel of the minor branch they are on and this should happen almost automatically always when we release one. In fact I expressed it a few times in our discsussin with our users like "why the hell you **just** upgraded to 2.4.0 when there is 2.4.3 for 5 months already? This makes absolutely no sense".. And yes - this makes no sense only if we are super-careful that this kind of change does not introduce new bugs. And reviewability is one of the key parts of it. > I think the argument to not include it in 2.6.1 isn't that it is a feature, but rather that it's not a bugfix. I have always assumed that the rationale for including only bugfixes in patch releases is because ideally patch releases should always make the thing more reliable, i.e. should fix things, and that things which aren't fixes and have some potential to cause breakage should be deferred. And a refactor such as this does doesn't fix anything and it could break things. Yes - I am also hesitating with cherry-picking optimisations (especially those involving import reshuffling) when they are not also fixing actual bug. This has hit us multiple times in the past. And even recently we had a problem where `--preload` flag implemented to fix problems in webserver caused something else (releoading FLASK API plugins to stop being reloaded during development - see issue here: https://github.com/apache/airflow/issues/30910). The original change WAS fixing an actual problem, and from what I see the fix broke onle a "developer" workflow, so not a big deal, however this one has a much bigger potential for breaking things. Ideally those kind of optimisations should be done late in the release cycle, shortly before we cut-off release branch. This makes it easier for everyone **I think**. Maybe a good solution (@o-nikolas maybe that is a good idea ???) will be to PARK this one, leave it open in DRAFT stage and come back to it say, in 2 months when we get close to cutting the 2.7 branch ? I think that would be much safer approach. Usually it is kinda expected that .0 release might contain some teething problems that are solved in .1 rather quickly. I know this is usually a pattern some of our users have (those that are more concerned about stability) that they wait for .1 release (but we also have other kinds of users who are eager to use more features - and those are the kind of users that are less "risk-averse" and they are testing such .0 releases quicker. I think adapting to those patterns of our different kinds of users is a good idea: a) bit risky changes that are likely to cause conflicts -> we wait with merging to "shortly before minor branch" and make them merged for the .0 release b) bug fixes that clearly fix problems and reviewable and low-risk -> merge them quickly and cherry-pick to .1+ That would be my proposal. -- 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]
