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

Make sure FirstPartyHelmDeploymentMapping doesn't get calculated if disabled (Cherry-pick of #21633) #21636

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

tdyas
Copy link
Contributor

@tdyas tdyas commented Nov 12, 2024

Option [helm-infer].deployment_dependencies was added in #21282. Unfortunately, I missed one place where FirstPartyHelmDeploymentMapping is requested:

@rule(desc="Prepare Helm deployment post-renderer", level=LogLevel.DEBUG)
async def prepare_post_renderer_for_helm_deployment(
    request: HelmDeploymentPostRendererRequest,
    union_membership: UnionMembership,
    docker_options: DockerOptions,
) -> SetupHelmPostRenderer:
    mapping = await Get(
        FirstPartyHelmDeploymentMapping, FirstPartyHelmDeploymentMappingRequest(request.field_set)
    )
    ...

Here I move this logic inside the rule, so that all consumers correctly handle the case when [helm-infer].deployment_dependencies is set to false

…isabled (pantsbuild#21633)

Option `[helm-infer].deployment_dependencies` was added in
pantsbuild#21282. Unfortunately, I missed
one place where `FirstPartyHelmDeploymentMapping` is requested:
```python
@rule(desc="Prepare Helm deployment post-renderer", level=LogLevel.DEBUG)
async def prepare_post_renderer_for_helm_deployment(
    request: HelmDeploymentPostRendererRequest,
    union_membership: UnionMembership,
    docker_options: DockerOptions,
) -> SetupHelmPostRenderer:
    mapping = await Get(
        FirstPartyHelmDeploymentMapping, FirstPartyHelmDeploymentMappingRequest(request.field_set)
    )
    ...
```
Here I move this logic inside the rule, so that all consumers correctly
handle the case when `[helm-infer].deployment_dependencies` is set to
`false`
@tdyas tdyas added category:bugfix Bug fixes for released features release-notes:not-required PR doesn't require mention in release notes labels Nov 12, 2024
@tdyas tdyas changed the title Make sure FirstPartyHelmDeploymentMapping doesn't get calculated if disabled (#21633) Make sure FirstPartyHelmDeploymentMapping doesn't get calculated if disabled (Cherry-pick of #21633) Nov 12, 2024
@tdyas
Copy link
Contributor Author

tdyas commented Nov 12, 2024

This PR did not automatically cherry pick to the 2.24.x branch because the 2.24.x milestone had not been created yet (it is now).

@tdyas
Copy link
Contributor Author

tdyas commented Nov 12, 2024

cc @grihabor

@grihabor
Copy link
Contributor

shall we merge it now?

@tdyas
Copy link
Contributor Author

tdyas commented Nov 13, 2024

shall we merge it now?

I can't because it still needs a maintainer to approve.

@benjyw benjyw merged commit e3f1d83 into pantsbuild:2.24.x Nov 13, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:bugfix Bug fixes for released features release-notes:not-required PR doesn't require mention in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants