You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "chamikaramj (via GitHub)" <gi...@apache.org> on 2026/04/30 23:52:32 UTC
[PR] Support staging Python packages given via a URL [beam]
chamikaramj opened a new pull request, #38348:
URL: https://github.com/apache/beam/pull/38348
For example, with this Beam YAML users can provide packages that include Python providers via a URL instead of having to provide a local package or a PyPI package.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://github.com/apache/beam/blob/master/CONTRIBUTING.md#make-the-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI or the [workflows README](https://github.com/apache/beam/blob/master/.github/workflows/README.md) to see a list of phrases to trigger workflows.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support staging Python packages given via a URL [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #38348:
URL: https://github.com/apache/beam/pull/38348#discussion_r3191057142
##########
sdks/python/apache_beam/runners/portability/stager.py:
##########
@@ -701,14 +701,29 @@ def _remove_dependency_from_requirements(
return tmp_requirements_filename
@staticmethod
- def _extract_local_packages(requirements_file):
+ def _extract_local_packages(requirements_file, temp_dir):
local_deps = []
pypi_deps = []
with open(requirements_file, 'r') as fin:
+ staging_temp_dir = tempfile.mkdtemp(dir=temp_dir)
for line in fin:
dep = line.strip()
if os.path.exists(dep):
local_deps.append(dep)
+ elif urlparse(dep).scheme:
+ last_component = dep.rsplit('/', 1)[-1]
Review Comment:
The extraction of the filename from the URL using `rsplit('/', 1)[-1]` is fragile. It can result in invalid filenames if the URL contains query parameters or fragments (e.g., `?v=1`). Additionally, URLs with the same filename but different paths (e.g., `.../v1/pkg.tar.gz` and `.../v2/pkg.tar.gz`) will overwrite each other in the temporary directory.
Consider using `urlparse` to extract the path and `os.path.basename` for the filename to improve robustness.
```python
parsed_url = urlparse(dep)
if os.path.exists(dep):
local_deps.append(dep)
elif parsed_url.scheme:
last_component = os.path.basename(parsed_url.path)
```
##########
sdks/python/apache_beam/runners/portability/stager.py:
##########
@@ -701,14 +701,29 @@ def _remove_dependency_from_requirements(
return tmp_requirements_filename
@staticmethod
- def _extract_local_packages(requirements_file):
+ def _extract_local_packages(requirements_file, temp_dir):
local_deps = []
pypi_deps = []
with open(requirements_file, 'r') as fin:
+ staging_temp_dir = tempfile.mkdtemp(dir=temp_dir)
for line in fin:
dep = line.strip()
if os.path.exists(dep):
local_deps.append(dep)
+ elif urlparse(dep).scheme:
+ last_component = dep.rsplit('/', 1)[-1]
+ if not last_component:
+ _LOGGER.warning(
+ 'Extra package %s has an unexpected format hence will not be downloaded locally.'
+ % dep)
+ continue
+ # Download remote package.
+ _LOGGER.info(
+ 'Downloading remote extra package: %s locally before staging',
+ dep)
+ local_file_path = FileSystems.join(staging_temp_dir, last_component)
+ Stager._download_file(dep, local_file_path)
+ local_deps.append(local_file_path)
Review Comment:
There is a bug in the requirements thinning logic. The `Stager._remove_dependency_from_requirements` method (called after this loop) uses the items in `local_deps` to identify which lines to remove from the original requirements file.
By appending the local temporary path (`local_file_path`) to `local_deps` instead of the original URL string (`dep`), the thinning logic will fail to match the URL line in the file. Consequently, the URL will remain in the thinned requirements file, causing the worker's pip to attempt to download it again, which defeats the purpose of staging the package locally.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support staging Python packages given via a URL [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on PR #38348:
URL: https://github.com/apache/beam/pull/38348#issuecomment-4356993388
> [!WARNING]
> Gemini encountered an error creating the summary. You can try again by commenting `/gemini summary`.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support staging Python packages given via a URL [beam]
Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on PR #38348:
URL: https://github.com/apache/beam/pull/38348#issuecomment-4490067487
waiting on author
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support staging Python packages given via a URL [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38348:
URL: https://github.com/apache/beam/pull/38348#issuecomment-4478199819
Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @tvalentyn for label python.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support staging Python packages given via a URL [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #38348:
URL: https://github.com/apache/beam/pull/38348#discussion_r3191478737
##########
sdks/python/apache_beam/runners/portability/stager.py:
##########
@@ -701,14 +701,30 @@ def _remove_dependency_from_requirements(
return tmp_requirements_filename
@staticmethod
- def _extract_local_packages(requirements_file):
+ def _extract_local_packages(requirements_file, temp_dir):
local_deps = []
pypi_deps = []
with open(requirements_file, 'r') as fin:
+ staging_temp_dir = tempfile.mkdtemp(dir=temp_dir)
for line in fin:
dep = line.strip()
+ parsed_url = urlparse(dep)
Review Comment:
The `urlparse` function is used here but it is not imported in this file. This will cause a `NameError` at runtime. Please add `from urllib.parse import urlparse` to the imports section of the file.
##########
sdks/python/apache_beam/runners/portability/stager.py:
##########
@@ -701,14 +701,30 @@ def _remove_dependency_from_requirements(
return tmp_requirements_filename
@staticmethod
- def _extract_local_packages(requirements_file):
+ def _extract_local_packages(requirements_file, temp_dir):
local_deps = []
pypi_deps = []
with open(requirements_file, 'r') as fin:
+ staging_temp_dir = tempfile.mkdtemp(dir=temp_dir)
for line in fin:
dep = line.strip()
+ parsed_url = urlparse(dep)
if os.path.exists(dep):
local_deps.append(dep)
+ elif parsed_url.scheme:
+ last_component = os.path.basename(parsed_url.path)
+ if not last_component:
+ _LOGGER.warning(
+ 'Extra package %s has an unexpected format hence will not be downloaded locally.'
+ % dep)
+ continue
+ # Download remote package.
+ _LOGGER.info(
+ 'Downloading remote extra package: %s locally before staging',
+ dep)
+ local_file_path = FileSystems.join(staging_temp_dir, last_component)
+ Stager._download_file(dep, local_file_path)
+ local_deps.append(local_file_path)
Review Comment:
This implementation has two potential issues:
1. **Filename Collision**: If multiple URLs have the same basename (e.g., `.../v1/pkg.tgz` and `.../v2/pkg.tgz`), they will overwrite each other in the temporary directory, and `local_deps` will contain the same path twice.
2. **Unsupported Schemes**: `FileSystems.open` (used in `_download_file`) does not support VCS schemes like `git+https`. If such a URL is encountered, the staging process will crash.
Adding a unique prefix to the filename and wrapping the download in a try-except block to fall back to `pypi_deps` (letting `pip` handle it on the worker) makes this more robust.
```suggestion
local_file_path = FileSystems.join(
staging_temp_dir, f"{len(local_deps)}_{last_component}")
try:
Stager._download_file(dep, local_file_path)
local_deps.append(local_file_path)
except Exception as e:
_LOGGER.warning(
'Failed to download remote package %s, falling back to pip: %s',
dep,
e)
pypi_deps.append(dep)
```
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support staging Python packages given via a URL [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38348:
URL: https://github.com/apache/beam/pull/38348#issuecomment-4450824762
Reminder, please take a look at this pr: @claudevdm
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support staging Python packages given via a URL [beam]
Posted by "chamikaramj (via GitHub)" <gi...@apache.org>.
chamikaramj commented on PR #38348:
URL: https://github.com/apache/beam/pull/38348#issuecomment-4382899154
/gemini review
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support staging Python packages given via a URL [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38348:
URL: https://github.com/apache/beam/pull/38348#issuecomment-4382014967
Assigning reviewers:
R: @claudevdm for label python.
Note: If you would like to opt out of this review, comment `assign to next reviewer`.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
The PR bot will only process comments in the main thread (not review comments).
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support staging Python packages given via a URL [beam]
Posted by "claudevdm (via GitHub)" <gi...@apache.org>.
claudevdm commented on PR #38348:
URL: https://github.com/apache/beam/pull/38348#issuecomment-4382361342
/gemini review
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support staging Python packages given via a URL [beam]
Posted by "chamikaramj (via GitHub)" <gi...@apache.org>.
chamikaramj commented on code in PR #38348:
URL: https://github.com/apache/beam/pull/38348#discussion_r3191453357
##########
sdks/python/apache_beam/runners/portability/stager.py:
##########
@@ -701,14 +701,29 @@ def _remove_dependency_from_requirements(
return tmp_requirements_filename
@staticmethod
- def _extract_local_packages(requirements_file):
+ def _extract_local_packages(requirements_file, temp_dir):
local_deps = []
pypi_deps = []
with open(requirements_file, 'r') as fin:
+ staging_temp_dir = tempfile.mkdtemp(dir=temp_dir)
for line in fin:
dep = line.strip()
if os.path.exists(dep):
local_deps.append(dep)
+ elif urlparse(dep).scheme:
+ last_component = dep.rsplit('/', 1)[-1]
Review Comment:
Done.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support staging Python packages given via a URL [beam]
Posted by "chamikaramj (via GitHub)" <gi...@apache.org>.
chamikaramj closed pull request #38348: Support staging Python packages given via a URL
URL: https://github.com/apache/beam/pull/38348
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support staging Python packages given via a URL [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38348:
URL: https://github.com/apache/beam/pull/38348#issuecomment-4357328422
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support staging Python packages given via a URL [beam]
Posted by "chamikaramj (via GitHub)" <gi...@apache.org>.
chamikaramj commented on code in PR #38348:
URL: https://github.com/apache/beam/pull/38348#discussion_r3191463960
##########
sdks/python/apache_beam/runners/portability/stager.py:
##########
@@ -701,14 +701,29 @@ def _remove_dependency_from_requirements(
return tmp_requirements_filename
@staticmethod
- def _extract_local_packages(requirements_file):
+ def _extract_local_packages(requirements_file, temp_dir):
local_deps = []
pypi_deps = []
with open(requirements_file, 'r') as fin:
+ staging_temp_dir = tempfile.mkdtemp(dir=temp_dir)
for line in fin:
dep = line.strip()
if os.path.exists(dep):
local_deps.append(dep)
+ elif urlparse(dep).scheme:
+ last_component = dep.rsplit('/', 1)[-1]
+ if not last_component:
+ _LOGGER.warning(
+ 'Extra package %s has an unexpected format hence will not be downloaded locally.'
+ % dep)
+ continue
+ # Download remote package.
+ _LOGGER.info(
+ 'Downloading remote extra package: %s locally before staging',
+ dep)
+ local_file_path = FileSystems.join(staging_temp_dir, last_component)
+ Stager._download_file(dep, local_file_path)
+ local_deps.append(local_file_path)
Review Comment:
I don't think this analysis is correct. AFAICT `Stager._remove_dependency_from_requirements` is called one time specifically to remove the "apache-beam" package. Not as a way to trim the requirements file based on local deps.
https://github.com/apache/beam/blob/efe4e941939a77275146ecceb01cd74b28555286/sdks/python/apache_beam/runners/portability/stager.py#L755
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support staging Python packages given via a URL [beam]
Posted by "chamikaramj (via GitHub)" <gi...@apache.org>.
chamikaramj commented on code in PR #38348:
URL: https://github.com/apache/beam/pull/38348#discussion_r3191494534
##########
sdks/python/apache_beam/runners/portability/stager.py:
##########
@@ -701,14 +701,30 @@ def _remove_dependency_from_requirements(
return tmp_requirements_filename
@staticmethod
- def _extract_local_packages(requirements_file):
+ def _extract_local_packages(requirements_file, temp_dir):
local_deps = []
pypi_deps = []
with open(requirements_file, 'r') as fin:
+ staging_temp_dir = tempfile.mkdtemp(dir=temp_dir)
for line in fin:
dep = line.strip()
+ parsed_url = urlparse(dep)
Review Comment:
Incorrect analysis (import is available at the top of the file).
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org