Revert "Suspend Apache Beam Provider due to grpcio limitation (#61926)"#66952
Revert "Suspend Apache Beam Provider due to grpcio limitation (#61926)"#66952olegkachur-e wants to merge 1 commit into
Conversation
3fbd131 to
01bc19c
Compare
|
@olegkachur-e A few things need addressing before review — see our Pull Request quality criteria.
No rush. Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you. |
1647831 to
794fc1a
Compare
| # For Python>=3.11, the apache-beam package depends on envoy-data-plane. | ||
| # Currently, there is only one compatible version(1.0.3) of envoy-data-plane available for these | ||
| # Python versions, and it explicitly requires a prerelease version of betterproto. | ||
| "betterproto==2.0.0b6; python_version >= '3.11'", |
There was a problem hiding this comment.
I am not a fan of this.
this is a beta version from 2023. This is not a case where in a few days/weeks things will change and a stable version would be released, is it?
I am not in the details of this so I will leave @potiuk the call here but from my point of view we should not deliever stable release with depenedency on beta packages.
Who gurentees that tommorow the author won't remove them from PyPi with a thought that there won't be any production impact on anyone?
There was a problem hiding this comment.
Not a big fan either.
But I think thee newer version of envoy-data-plane has been questioned by others for example here: apache/beam#37854
And I see that this is what latest beam has:
envoy-data-plane<2,>=1.0.3; python_version >= "3.11"
And the 2.1.0 version of envoy-data-plane uses already betterproto2 - which is a maintained replacement of betterproto.
So maybe we can ask apache-beam to upgrade to the envoy-data-plane>=2 ?
There was a problem hiding this comment.
I am not a big fan of this idea either.
The reason why I decided to add it was in that this beta version exists from 2023 and I am not sure that it will be release in the future. One more point for me to added it was that I noticed that Apache Beam SDK use this beta version in there generated requirements for containers. And I had idea that if they already had hardcoded it in apache-beam requirements, from which our provider already depends, maybe it makes sense temporary hardcoded it in provider as well for fixing constrains problem and unsuspend provider.
There was a problem hiding this comment.
We can't tell beam what to do from thier side but Airflow should not do this.
There was a problem hiding this comment.
Created this PR in beam to try and tackle this issue:
apache/beam#39213
|
I commented in the issue. |
|
Quickest fix: git fetch upstream main && git rebase upstream/main
rm uv.lock && uv lock
git add uv.lock && git rebase --continue
git push --force-with-leaseAutomated nudge — ignore if you're not ready to rebase. This comment is updated in place on future |
…#61926)" This reverts commit 917abea. - Remove hacks regarding the beam provider suspension, as they are not relevant anymore. ISSUE: apache#66551
794fc1a to
8a4a569
Compare
potiuk
left a comment
There was a problem hiding this comment.
Thanks for pushing on the un-suspension. Two things before this can move forward — neither is a code-quality issue with the revert itself:
- Rebase needed: the PR is currently conflicting with
main(uv.lock/pyproject.toml). It'll need a rebase before CI is meaningful and before it can merge. - Tracking issue for the prerelease pin: the revert hard-pins a prerelease —
betterproto==2.0.0b6; python_version >= "3.11"— as a workaround for theenvoy-data-plane1.0.3 constraint. Per the project convention, a workaround/version-pin like this needs a GitHub tracking issue, referenced by full URL both in the PR body and as a comment at the pin site, describing the follow-up (moving apache-beam ontoenvoy-data-plane>=2/betterproto2so the prerelease pin can be dropped). The explanatory comment that's there is good, but it should link the tracking issue so the pin actually gets removed later rather than becoming permanent.
Not approving yet — once it's rebased and the pin has a tracking-issue link, happy to take another look.
This review was drafted by an AI-assisted tool and confirmed by an Airflow maintainer. The findings below are observations, not blockers; an Airflow maintainer — a real person — will take the next look at the PR. If you think a finding is mis-applied, please reply on the PR and a maintainer will weigh in.
More on how Airflow handles maintainer review: contributing-docs/05_pull_requests.rst.
| # For Python>=3.11, the apache-beam package depends on envoy-data-plane. | ||
| # Currently, there is only one compatible version(1.0.3) of envoy-data-plane available for these | ||
| # Python versions, and it explicitly requires a prerelease version of betterproto. | ||
| "betterproto==2.0.0b6; python_version >= '3.11'", |
There was a problem hiding this comment.
This exact-prerelease pin is a workaround for the envoy-data-plane 1.0.3 constraint, so per the project's workaround convention it needs a GitHub tracking issue — linked by full URL here at the pin site (not a bare #NNNNN) and referenced in the PR body — capturing the follow-up: getting apache-beam onto envoy-data-plane>=2 / betterproto2 so this ==2.0.0b6 pin can be removed. Without that, an exact prerelease pin tends to quietly become permanent.
|
Please also amend commit message to a meaningful one. Revert old commit is not very informative. |
This reverts commit 917abea.
Was generative AI tooling used to co-author this PR?
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.Important
🛠️ Maintainer triage note for @olegkachur-e · by
@potiuk· 2026-07-02 17:46 UTCSome review feedback from
@eladkalis waiting on you:@eladkalneed a reply or a fix.The ball is in your court — you've been assigned to this PR. Reply or push a fix in each thread, then mark them resolved.
Automated triage — may be imperfect; a maintainer takes the next look.