Skip to content

Conversation

@Gabriella439
Copy link
Collaborator

Related to dhall-lang/dhall-kubernetes#136

In the long-run I plan to fix all of the dhall-kubernetes special-case
logic by undoing this logic:

alwaysRequired = Set.fromList $ FieldName <$> [ "apiVersion", "kind", "metadata"]

… which would make apiVersion/kind Optional across the board.
However, until then, this change makes them selectively Optional
only for PersistentVolumeClaim.

Related to dhall-lang/dhall-kubernetes#136

In the long-run I plan to fix all of the `dhall-kubernetes` special-case
logic by undoing this logic:

https://github.com/dhall-lang/dhall-haskell/blob/9dca2b8129bfec47afa069d5d1008448dcd2f873/dhall-openapi/src/Dhall/Kubernetes/Convert.hs#L44

… which would make `apiVersion`/`kind` `Optional` across the board.
However, until then, this change makes them selectively `Optional`
only for `PersistentVolumeClaim`.
@mergify mergify bot merged commit f77549a into master Sep 2, 2020
@mergify mergify bot deleted the gabriel/PersistentVolumeClaim branch September 2, 2020 18:05
@PierreR
Copy link
Contributor

PierreR commented Sep 17, 2020

@Gabriel439 I am wondering if the fix is complete. It looks like the generated default file for io.k8s.api.core.v1.PersistentVolumeClaim.dhall is:

{ apiVersion = "v1"
, kind = "PersistentVolumeClaim"
, spec = None ./../types/io.k8s.api.core.v1.PersistentVolumeClaimSpec.dhall
, status = None ./../types/io.k8s.api.core.v1.PersistentVolumeClaimStatus.dhall
}

and it does not match the associated type:

{ metadata : ./io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta.dhall
, apiVersion : Optional Text
, kind : Optional Text
, spec : Optional ./io.k8s.api.core.v1.PersistentVolumeClaimSpec.dhall
, status : Optional ./io.k8s.api.core.v1.PersistentVolumeClaimStatus.dhall
}

@Gabriella439
Copy link
Collaborator Author

@PierreR: Yeah, it is a bug. I'm currently on vacation this week, but I'll fix it afterwards

@PierreR
Copy link
Contributor

PierreR commented Sep 18, 2020

@Gabriel439 Thanks. It is not urgent at all.

Gabriella439 added a commit that referenced this pull request Sep 21, 2020
… as caught by @PierreR in #2019 (comment)

The issue was that the logic for generating the default for a type had a
special case for the `apiVersion` / `kind` fields, which was overriding
the correct `None` defaults.  This change fixes that by switching the
priority so that if the `None` defaults are present then they take
precedence over the hard-coded defaults.
@Gabriella439
Copy link
Collaborator Author

@PierreR: The fix is up here: #2053

Gabriella439 added a commit that referenced this pull request Oct 2, 2020
… as caught by @PierreR in #2019 (comment)

The issue was that the logic for generating the default for a type had a
special case for the `apiVersion` / `kind` fields, which was overriding
the correct `None` defaults.  This change fixes that by switching the
priority so that if the `None` defaults are present then they take
precedence over the hard-coded defaults.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants