Helm deployment chart field#19234
Conversation
|
Worth mentioning that this PR would make it easier to address #17933 |
lilatomic
left a comment
There was a problem hiding this comment.
Looks neat! I think that a helm_deployment referencing a helm_chart feels similar to how a pex_binary often references python_sources
| # ----------------------------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| class HelmDeploymentChartField(StringField, AsyncFieldMixin): |
There was a problem hiding this comment.
Can you talk about why we don't use SpecialCasedDependencies here? Is it because we might be depending on a Helm chart from a repository, which wouldn't be a Pants dependency? something like helm_deployment(chart="bitnami/wordpress")
I'm wondering about the reasoning so I can think it through for Terraform.
There was a problem hiding this comment.
Well, it's the same reason of moving away from using the dependencies field: both take a list of values while I would like to enforce that is one, and only one mandatory value.
There was a problem hiding this comment.
That makes sense, thanks. I think that'll be useful for Terraform as well
| ) | ||
| targets.append(wrapped_target.target) | ||
| else: | ||
| explicit_dependencies = await Get( |
There was a problem hiding this comment.
Will this branch become unneeded when chart is mandatory?
There was a problem hiding this comment.
Yeah, that branch and the errors related to this logic go away by having a more specific field.
All that will be removed for 2.19
| HelmDeploymentChartField, | ||
| HelmDeploymentReleaseNameField, | ||
| HelmDeploymentDependenciesField, | ||
| HelmDeploymentSourcesField, |
There was a problem hiding this comment.
Does having a helm_deployment and a helm_chart in the same directory work? Both of their sources fields pull in the same files, so they're owned by more than one target, but the files are the same so merging the digest should work.
There was a problem hiding this comment.
It could work if the user specifies the sources field for each target in a way that they don't overlap.
However the recommended approach is to have them in separate folders. It's also good practice to do so.
Co-authored-by: Benjy Weinberger <benjyw@gmail.com>
Co-authored-by: Benjy Weinberger <benjyw@gmail.com>
This updates `changelog.py` to write the (non-internal) changes to the relevant release notes file directly, rather than requiring a human to copy-paste it in. For now, the file is just mutated, without committing. The internal changes are still printed for the human to deal with. For instance, `pants run src/python/pants_release/changelog.py -- --prior 2.18.0.dev1 --new 2.18.0.dev2` adds a new section to `src/python/pants/notes/2.18.x.md`: ```diff diff --git a/src/python/pants/notes/2.18.x.md b/src/python/pants/notes/2.18.x.md index e648a45..d6668a24b1 100644 --- a/src/python/pants/notes/2.18.x.md +++ b/src/python/pants/notes/2.18.x.md @@ -1,5 +1,35 @@ # 2.18.x Release Series +## 2.18.0.dev2 (Jun 14, 2023) + +### New Features + +* Include complete platforms for FaaS environments for more reliable building ([#19253](#19253)) + +* Add experimental support for Rustfmt ([#18842](#18842)) + +* Helm deployment chart field ([#19234](#19234)) + +### Plugin API Changes + +* Replace `include_special_cased_deps` flag with `should_traverse_deps_predicate` ([#19272](#19272)) + +### Bug Fixes + +* Raise an error if isort can't read a config file ([#19294](#19294)) + +* Improve handling of additional files in Helm unit tests ([#19263](#19263)) + +* Add taplo to the release ([#19258](#19258)) + +* Handle `from foo import *` wildcard imports in Rust dep inference parser ([#19249](#19249)) + +* Support usage of `scala_artifact` addresses in `scalac_plugin` targets ([#19205](#19205)) + +### Performance + +* `scandir` returns `Stat`s relative to its directory. ([#19246](#19246)) + ## 2.18.0.dev1 (Jun 02, 2023) ### New Features ``` This also moves it into the new `pants_release` root, adds some basic tests, and has it fetch the relevant branch directly, rather than prompting the user to do so. It also pulls out a `git.py` support module with helpers for exec-ing `git` as an external process. The commits are individually reviewable. This is a step towards automating more of the "start release" process, per #19279. After this core refactoring of the functionality, I think the next step is to combine it with the CONTRIBUTORS update and version bumping, and then after that string it all together on CI so that starting a release is fully automated.
Adds a new
chartfield to thehelm_deploymenttarget and moves await from the recommendation of explicitly listing the chart among the deployment dependencies. The field will be made required in a later version.The motivation of this change is to make declaring Helm deployments more intentional and less error prone to declare. With the addition of #19185, explicitly declaring a dependency from a Helm deployment into a Terraform deployment is to be expected so this change will also make identifying the actual chart easier from a quick glance at a BUILD file.
The referenced chart will still be listed as a dependency as it will be included in the inferred dependencies list.