Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Client Version 2.6.1 #80

Merged
merged 2 commits into from
May 17, 2023
Merged

Add Client Version 2.6.1 #80

merged 2 commits into from
May 17, 2023

Conversation

ephraimbuddy
Copy link
Contributor

No description provided.

…_timeout (#53) (#76)

(cherry picked from commit f32517a)

Co-authored-by: David Katz <[email protected]>
(cherry picked from commit ddd6fc0)
@ephraimbuddy
Copy link
Contributor Author

We only have one fix in this release and I wonder if it's worth releasing with just this fix?

Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also agree, maybe we should relax a bit the policy for releasing clients because:

  • Current policy means releasing clients for almost every airflow patch, which put some additional pressure on RM (extra process + votes etc.)
  • It’s hard to actually release them every single time as core airflow process is getting bigger and bigger. Can take a whole week to get the release out.

Maybe for patch we can update the policy to “can release clients at the discretion of RM”, and keep what we have for major/minor (systematic release of clients)

@potiuk
Copy link
Member

potiuk commented May 17, 2023

Also agree, maybe we should relax a bit the policy for releasing clients because:

  • Current policy means releasing clients for almost every airflow patch, which put some additional pressure on RM (extra process + votes etc.)
  • It’s hard to actually release them every single time as core airflow process is getting bigger and bigger. Can take a whole week to get the release out.

Are you sure? I think this one is only needed because we actually had an API related fix (Fix Pool schema OpenAPI spec (#30973) - but this happens very rarely after we release the .0 version. I think this one is an exception rather than a rule.

The current policy is quite strict that we should only release the API clients if there is an API change - so this time 2.6.1 Airflow is accompanied with 2.6.1 Python client, but it does not have to be the case (and actually very rarely was I think).

I can easily imagine that we won't find any API changes that we need to release in 2.6.2, 2.6.3, 2.6.4 ....

@potiuk
Copy link
Member

potiuk commented May 17, 2023

BTW. The way to control it for RM is not to cherry-pick API changes to the v2_* branch even if we find a bug. So I think if we find an API change that is not worth cherry-picking, we won't cherry-pick (and won't release the API client). But if we think that the API change is so important that we should cherry-pick it, then by all means - we should also release the python client that has that change.

@ephraimbuddy ephraimbuddy merged commit 8d86867 into main May 17, 2023
@ephraimbuddy ephraimbuddy deleted the release261 branch May 17, 2023 09:41
@pierrejeambrun
Copy link
Member

I didn't think of it that way but indeed RM can control this when cutting the patch release. This gives us the flexibility I was hoping for in my proposal.

Thanks

@potiuk
Copy link
Member

potiuk commented May 17, 2023

I didn't think of it that way but indeed RM can control this when cutting the patch release. This gives us the flexibility I was hoping for in my proposal.

IMHO, we are responsible for everything we decided to release: if we decided to release an API change that impacts the client, then not realeasing client that supports that change is a bit of half-baked-cake.

I always think of this from the user perspective.

If a user gets our released software (client) in the latest version and use it with our other software (airflow) in latest version and gets a failure and opens an issue that it does not work - because of compatibility issues. What do we do ? Should we fix it? If the answer is yes, then if we knowingly decide not to release such change, we are actually knowingly causing a problem for that user (which we could have prevented by releasing the client).

There is one thing to do it accidentally, and another thing to decide to do it deliberately (which in this case we know if the user uses the latest client they will have a problem with the new Airflow.

For me this is no brainer.

But there is also another decision point (which is much closer to your point and I am fine with it). If the RM will decide that the change in the API will not cause the user to suffer - then it's fine not to release the client.

For example if the Open API change is fully backwards compatible and the old client will work with the new API and we can reliiably confirm it, that's fine - let's not release the client. For example if this is just clarification of the type returned, that will not cause any change on the client side, or when there is a change in description/comment in the Open API (which does not impact the API client implementation) - then we can decide not to release the client.

But I think in this case, the scope of the change is such that either user will not be able to do something available in Airlfow 2.6.1 or it will fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants