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

Migrates generate-lockfiles to USES_ENVIRONMENTS. #17300

Merged

Conversation

chrisjrn
Copy link
Contributor

This is a least-effort attempt at getting generate-lockfiles into a position where individual backend implementations can select an environment in which to generate the lockfiles.

The request generation phase runs in the default local environment, and is responsible for adding an environments field to the GenerateLockfile request object.

If a request does not specify environments, the GenerateLockfile request is run in the default local environment. If a single environment is specified, that environment is to run the request. If more than one environment is specified, a warning is displayed, and the request runs in either the default local environment (if present in the request), or an arbitrary environment.

This approach will give us an avenue in the future to run the request in all the specified environments, and add behaviour to somehow merge the resulting lockfiles if different, since the eventual act of writing the lockfiles to disk happens in the rule goal code.

Addresses #17129

@chrisjrn chrisjrn marked this pull request as ready for review October 20, 2022 21:51
Copy link
Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Thanks!

src/python/pants/core/goals/generate_lockfiles.py Outdated Show resolved Hide resolved
@@ -407,6 +430,25 @@ async def generate_lockfiles_goal(
return GenerateLockfilesGoal(exit_code=0)


def _preferred_environment(request: GenerateLockfile, default: EnvironmentName) -> EnvironmentName:

if not isinstance(request, GenerateLockfileWithEnvironments):
Copy link
Member

Choose a reason for hiding this comment

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

An alternative to subtyping might be to give GenerateLockfile a field which is optional (for now?)

  environments: tuple[EnvironmentName, ...] |  None = None

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alas, it's not possible to have an optional value there, because GenerateLockfile has subclasses with required parameters. dataclasses doesn't make this case easy.

src/python/pants/core/goals/generate_lockfiles.py Outdated Show resolved Hide resolved
Copy link
Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Thank you!

@chrisjrn chrisjrn merged commit 1631e95 into pantsbuild:main Oct 28, 2022
@chrisjrn chrisjrn deleted the chrisjrn/17129-generate-lockfiles-attempt-2 branch October 28, 2022 16:52
@stuhood stuhood mentioned this pull request Oct 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:internal CI, fixes for not-yet-released features, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants