-
-
Notifications
You must be signed in to change notification settings - Fork 643
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
Updates publish
to USES_ENVIRONMENTS
#17313
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,11 +29,11 @@ async def publish_example(request: PublishToMyRepoRequest, ...) -> PublishProces | |
|
||
from typing_extensions import final | ||
|
||
from pants.core.goals.package import BuiltPackage, PackageFieldSet | ||
from pants.core.goals.package import BuiltPackage, EnvironmentAwarePackageRequest, PackageFieldSet | ||
from pants.engine.addresses import Address | ||
from pants.engine.collection import Collection | ||
from pants.engine.console import Console | ||
from pants.engine.environment import EnvironmentName | ||
from pants.engine.environment import ChosenLocalEnvironmentName, EnvironmentName | ||
from pants.engine.goal import Goal, GoalSubsystem | ||
from pants.engine.process import InteractiveProcess, InteractiveProcessResult | ||
from pants.engine.rules import Effect, Get, MultiGet, collect_rules, goal_rule, rule | ||
|
@@ -180,11 +180,13 @@ def activated(cls, union_membership: UnionMembership) -> bool: | |
|
||
class Publish(Goal): | ||
subsystem_cls = PublishSubsystem | ||
environment_behavior = Goal.EnvironmentBehavior.LOCAL_ONLY # TODO(#17129) — Migrate this. | ||
environment_behavior = Goal.EnvironmentBehavior.USES_ENVIRONMENTS | ||
|
||
|
||
@goal_rule | ||
async def run_publish(console: Console, publish: PublishSubsystem) -> Publish: | ||
async def run_publish( | ||
console: Console, publish: PublishSubsystem, local_environment: ChosenLocalEnvironmentName | ||
) -> Publish: | ||
target_roots_to_package_field_sets, target_roots_to_publish_field_sets = await MultiGet( | ||
Get( | ||
TargetRootsToFieldSets, | ||
|
@@ -242,7 +244,10 @@ async def run_publish(console: Console, publish: PublishSubsystem) -> Publish: | |
continue | ||
|
||
logger.debug(f"Execute {pub.process}") | ||
res = await Effect(InteractiveProcessResult, InteractiveProcess, pub.process) | ||
res = await Effect( | ||
InteractiveProcessResult, | ||
{pub.process: InteractiveProcess, local_environment.val: EnvironmentName}, | ||
) | ||
if res.exit_code == 0: | ||
sigil = console.sigil_succeeded() | ||
status = "published" | ||
|
@@ -305,9 +310,12 @@ def default(self, o): | |
|
||
|
||
@rule | ||
async def package_for_publish(request: PublishProcessesRequest) -> PublishProcesses: | ||
async def package_for_publish( | ||
request: PublishProcessesRequest, local_environment: ChosenLocalEnvironmentName | ||
) -> PublishProcesses: | ||
packages = await MultiGet( | ||
Get(BuiltPackage, PackageFieldSet, field_set) for field_set in request.package_field_sets | ||
Get(BuiltPackage, EnvironmentAwarePackageRequest(field_set)) | ||
for field_set in request.package_field_sets | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stuhood this is where the package operation is potentially performed in a different environment. |
||
) | ||
|
||
for pkg in packages: | ||
|
@@ -320,8 +328,10 @@ async def package_for_publish(request: PublishProcessesRequest) -> PublishProces | |
publish = await MultiGet( | ||
Get( | ||
PublishProcesses, | ||
PublishRequest, | ||
field_set._request(packages), | ||
{ | ||
field_set._request(packages): PublishRequest, | ||
local_environment.val: EnvironmentName, | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stuhood This is where the publish processes are constructed, pinned to the local environment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, you're right! I forgot that |
||
) | ||
for field_set in request.publish_field_sets | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's likely that some
publish
implementations will need to pin the construction of the actual publish command to use back to the local environment... because otherwise they might consume environment variables / etc to construct it "for" a remote environment?i.e.: Do packaging/etc in some environment, but then construct a publish command for the local environment.
I'm not sure if that suggests that they should each need to figure out their own environment internally instead... but ... maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't that what's going on here? The packaging request is using the environment that's needed to successfully construct the package, but the publish command is run locally.