docs: Add release notes for 3.6.9#91
Conversation
Co-Authored-By: alex.s@prefect.io <ajstreed1@gmail.com>
Co-Authored-By: alex.s@prefect.io <ajstreed1@gmail.com>
Code Review by Qodo (Alpha)
1. `cachetools` mismatch across pyprojects
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| "anyio>=4.4.0,<5.0.0", | ||
| "asgi-lifespan>=1.0,<3.0", | ||
| "cachetools>=5.3,<7.0", | ||
| "cachetools>=5.3,<8.0", |
There was a problem hiding this comment.
1. cachetools mismatch across pyprojects 📘 Rule Violation
• The root pyproject.toml updates the client dependency constraint for cachetools to <8.0, but the corresponding entry in client/pyproject.toml remains <7.0. • This violates the requirement to keep client-side dependencies consistent across both pyproject files and can lead to mismatched builds or runtime behavior between the main project and the client package.
Agent Prompt
## Issue description
The root `pyproject.toml` updates the client dependency constraint for `cachetools` to `<8.0`, but `client/pyproject.toml` still constrains `cachetools` to `<7.0`. This violates the repo requirement that client dependency updates be applied to both pyproject files.
## Issue Context
The root dependency list explicitly notes that changes must be mirrored in `client/pyproject.toml` to prevent mismatched client builds.
## Fix Focus Areas
- pyproject.toml[42-47]
- client/pyproject.toml[26-32]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| await _pull_git_repository_with_retries(storage) | ||
|
|
||
| return dict(directory=str(storage.destination.relative_to(Path.cwd()))) | ||
| return dict(directory=str(storage.destination)) |
There was a problem hiding this comment.
2. Git_clone returns absolute path 🐞 Bug
• agit_clone/git_clone now return directory as an absolute path (Path.cwd()/…) instead of
the previously returned relative directory name.
• Existing step tests assert the output is the relative directory ("repo"), so this is a breaking
change to the step output contract.
• Absolute temp paths are also less usable for step-output templating because they vary by runtime
environment.
Agent Prompt
## Issue description
`agit_clone` and `git_clone` changed their returned `directory` value from a relative directory (e.g. `repo`) to an absolute path (e.g. `/tmp/.../repo`). This breaks existing tests and makes step outputs non-portable for templating.
## Issue Context
The `GitRepository.destination` path is computed under a storage base path (defaulting to `Path.cwd()`); step outputs are used downstream (and in tests) as stable relative paths.
## Fix Focus Areas
- src/prefect/deployments/steps/pull.py[88-101]
- src/prefect/deployments/steps/pull.py[209-224]
## Suggested fix
- Restore relative return semantics:
- Prefer `directory = str(storage.destination.relative_to(Path.cwd()))`
- If `relative_to` raises (edge cases), fall back to `os.path.relpath(storage.destination, Path.cwd())` or finally the absolute path.
- Keep `pull_from_remote_storage` behavior unchanged (it already returns relative).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
162
to
164
| copytree( | ||
| from_path, local_path, dirs_exist_ok=True, ignore=ignore_func, symlinks=True | ||
| from_path, local_path, dirs_exist_ok=True, ignore=ignore_func, symlinks=False | ||
| ) |
There was a problem hiding this comment.
3. Async get_directory dereferences symlinks 🐞 Bug
• LocalFileSystem.aget_directory now calls shutil.copytree(..., symlinks=False), which dereferences symlinks (copies targets) instead of preserving links. • This diverges from the sync implementation (get_directory uses symlinks=True) and from the documented/announced behavior to preserve symlinks. • Following symlinks can unexpectedly pull in content outside the intended directory (and can expand large trees), making async vs sync behavior inconsistent.
Agent Prompt
## Issue description
`LocalFileSystem.aget_directory` switched to `shutil.copytree(..., symlinks=False)` which dereferences symlinks. This conflicts with the sync implementation (`symlinks=True`) and the documented expectation to preserve symlinks.
## Issue Context
`LocalFileSystem.get_directory` is decorated with `@async_dispatch(aget_directory)` and will use `aget_directory` when called from async contexts, so this impacts common runtime paths.
## Fix Focus Areas
- src/prefect/filesystems.py[130-165]
- src/prefect/filesystems.py[167-210]
## Suggested fix
- Change async `copytree(... symlinks=False)` to `symlinks=True` to preserve symlinks.
- Consider adding/adjusting tests for LocalFileSystem symlink handling in async context to prevent regressions.
- Ensure both implementations use the same symlink behavior.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
503
to
507
| "prefect.deployments.steps.git_clone": { | ||
| "repository": self._url, | ||
| "branch": self._branch, | ||
| "clone_directory_name": self._name, | ||
| } |
There was a problem hiding this comment.
4. To_pull_step forces clone name 🐞 Bug
• GitRepository.to_pull_step() now always emits clone_directory_name, even when the name is just the auto-inferred default. • This prevents the pull step from using its documented inference behavior (“if not provided, infer from repository URL and branch”) and breaks existing tests that expect omission when using defaults. • The implementation currently cannot distinguish between an explicitly provided name and an inferred default, so the current fix likely overshoots and changes generated prefect.yaml semantics.
Agent Prompt
## Issue description
`GitRepository.to_pull_step()` always includes `clone_directory_name`, even when the name is just the inferred default. This contradicts the documented behavior of `git_clone` (infer name when not provided) and breaks tests expecting omission for default names.
## Issue Context
`GitRepository.__init__` currently sets `self._name = name or default_name`, losing whether `name` was explicitly passed. Fixing “name preservation” should not require always emitting `clone_directory_name` for inferred defaults.
## Fix Focus Areas
- src/prefect/runner/storage.py[149-190]
- src/prefect/runner/storage.py[501-544]
## Suggested fix
- Add an internal flag, e.g. `self._name_was_provided = name is not None`.
- Keep `self._name = name or default_name` for destination computation.
- In `to_pull_step`, include `clone_directory_name` only when `self._name_was_provided` is True (or equivalently retain the previous “omit when equals default_name” behavior).
- Update/restore the existing tests in `tests/runner/test_storage.py` accordingly (they currently assert omission for default names).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Benchmark PR from qodo-benchmark#251