Define Pydantic model to representing Kubernetes manifest #546
Conversation
…into feat/add-operations
| annotations: dict[str, str] | None = None | ||
| creation_timestamp: str | None = Field( | ||
| default=None, description="Timestamp in RFC3339 format" | ||
| ) | ||
| deletion_grace_period_seconds: int | None = None | ||
| deletion_timestamp: str | None = Field( | ||
| default=None, description="Timestamp in RFC3339 format" | ||
| ) | ||
| finalizers: list[str] | None = None | ||
| generate_name: str | None = None | ||
| generation: int | None = None | ||
| labels: dict[str, str] | None = None | ||
| managed_fields: list[ManagedFieldsEntry] | None = None | ||
| name: str | None = None | ||
| namespace: str | None = None | ||
| owner_references: list[OwnerReference] | None = None | ||
| resource_version: str | None = None | ||
| self_link: str | None = Field( | ||
| default=None, | ||
| description="Deprecated field, not populated by Kubernetes in modern versions", | ||
| ) | ||
| uid: str | None = None |
There was a problem hiding this comment.
do we need all of those? I feel like many have no meaning to KPOps
There was a problem hiding this comment.
They are a representation of the lightkube ObjectMeta. They are documented here:
https://gtsystem.github.io/lightkube-models/1.19/models/meta_v1/#objectmeta
We can have them removed and allow extra fields.
There was a problem hiding this comment.
yeah, I think I would prefer a reduction to those fields that are most likely to be needed in KPOps
| generator = kpops.manifest_deploy( | ||
| RESOURCE_PATH / "manifest-pipeline" / PIPELINE_YAML, | ||
| environment="development", | ||
| ) | ||
| assert isinstance(generator, Iterator) | ||
| resources = list(generator) | ||
| assert len(resources) == 2 | ||
| snapshot.assert_match(yaml.dump_all(resources), "resources") |
There was a problem hiding this comment.
the new isinstance check is great, but I think we would still need some sort of snapshot testing as well. how about we just save the output of print_yaml(manifest.model_dump())?
There was a problem hiding this comment.
I found that a bit overkill for this test. The test should check if the KPOps manifest api works as expected or not. Printing the YAML would be done by the CLI, and that is tested in the other test cases. But if you think it make sense I would capture the stdout and add a snapshot :)
There was a problem hiding this comment.
I agree with you. we should probably split that up into two separate tests. In the API test we run assertions on the returned KubernetesManifest models and in the CLI test we do the snapshot test of the output. Let's do it in a followup then
Closes #548