Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Pass self through property functions #78

Merged
merged 1 commit into from
Sep 27, 2017
Merged

Conversation

hausdorff
Copy link
Contributor

@hausdorff hausdorff commented Sep 22, 2017

The net effect of these changes is that something like this is now possible:

local k = import "k.libsonnet";
local deployment = k.apps.v1beta1.deployment;
local container = k.apps.v1beta1.deployment.mixin.spec.template.spec.containersType;
local port = container.portsType;

local c = container
  .new("nginx-depl", "nginx")
  .withPorts(port.new(80));

deployment.new("nginx-depl", 1, c, {app: "nginx-depl"}) +
deployment.mixin.metadata
  .withName("foo")
  .withNamespace("bar")

@hausdorff hausdorff changed the title WIP: Pass self through property functions Pass self through property functions Sep 22, 2017
Copy link
Contributor

@jessicayuen jessicayuen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment, otherwise lgtm.

body = fmt.Sprintf("{%s+: %s}", fieldName, paramName)
setterBody = fmt.Sprintf("{%s: %s}", fieldName, paramName)
mixinBody = fmt.Sprintf("{%s+: %s}", fieldName, paramName)
emitMixin = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

emitMixin is set to true in both cases, can you take it out of the conditional? Same with above line 838.

@jbeda
Copy link
Contributor

jbeda commented Sep 26, 2017

This is super exciting! LGTM

Currently the `+` pattern is used prodigiously by ksonnet-lib. For
example, we might build up a container object with something like:

    container.new("nginx-depl", "nginx") +
    container.ports(port.new(80))

This commit introduces new "helper" construct that greatly cuts down on
verbosity, so that something like the following is possible:

  container
    .new("nginx-depl", "nginx")
    .withPorts(port.new(80))

This is supported, essentially, by having every method return `self`.
For example, in the case of `metadata.name`, we would have something
like the example below.

  name(name):: self + __metadataMixin({name: name}),

The essential nature of this change is to include the surrounding object
in the return, so that all the methods of (e.g.) `metadata` are
available on it.

Signed-off-by: Alex Clemmer <clemmer.alexander@gmail.com>
@hausdorff hausdorff merged commit ef07923 into ksonnet:master Sep 27, 2017
This was referenced Sep 27, 2017
@AntonioMeireles
Copy link

AntonioMeireles commented Oct 8, 2017

Hi again,

as of now it looks that current head is busted (for beta.3)

actually @hausdorff example above won't work at all ...

for it to behave, unless I'm missing something, it seems one has to manually patch ksonnet.beta.3/k8s.libsonnet along, at least, in those three places ...

diff --git a/ksonnet.beta.3/k8s.libsonnet b/ksonnet.beta.3/k8s.libsonnet
index 3b1bb7d..7cb3f16 100644
--- a/ksonnet.beta.3/k8s.libsonnet
+++ b/ksonnet.beta.3/k8s.libsonnet
@@ -348,7 +348,7 @@
       // Deployment enables declarative updates for Pods and ReplicaSets.
       deployment:: {
         local kind = {kind: "Deployment"},
-        new(name, replicas, containers, podLabels={app: name}):: apiVersion + kind + self.mixin.metadata.name(name) + self.mixin.spec.replicas(replicas) + self.mixin.spec.template.spec.containers(containers) + self.mixin.spec.template.metadata.labels(podLabels),
+        new(name, replicas, containers, podLabels={app: name}):: apiVersion + kind + self.mixin.metadata.withName(name) + self.mixin.spec.withReplicas(replicas) + self.mixin.spec.template.spec.withContainers(containers) + self.mixin.spec.template.metadata.withLabels(podLabels),
         mixin:: {
           // Standard object metadata.
           metadata:: {
@@ -865,7 +865,7 @@
       // The StatefulSet guarantees that a given network identity will always map to the same storage identity.
       statefulSet:: {
         local kind = {kind: "StatefulSet"},
-        new(name, replicas, containers, volumeClaims, podLabels={app: name}):: apiVersion + kind + self.mixin.metadata.name(name) + self.mixin.spec.replicas(replicas) + self.mixin.spec.template.spec.containers(containers) + self.mixin.spec.volumeClaimTemplates(volumeClaims) + self.mixin.spec.template.metadata.labels(podLabels),
+        new(name, replicas, containers, volumeClaims, podLabels={app: name}):: apiVersion + kind + self.mixin.metadata.withName(name) + self.mixin.spec.withReplicas(replicas) + self.mixin.spec.template.spec.withContainers(containers) + self.mixin.spec.withVolumeClaimTemplates(volumeClaims) + self.mixin.spec.template.metadata.withLabels(podLabels),
         mixin:: {
           //
           metadata:: {
@@ -4219,7 +4219,7 @@
       // Namespace provides a scope for Names. Use of multiple namespaces is optional.
       namespace:: {
         local kind = {kind: "Namespace"},
-        new(name):: apiVersion + kind + self.mixin.metadata.name(name),
+        new(name):: apiVersion + kind + self.mixin.metadata.withName(name),
         mixin:: {
           // Standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata
           metadata:: {

Thanks in advance!

@hausdorff
Copy link
Contributor Author

@AntonioMeireles I actually can't reproduce this issue. At the top of your k8s.libsonnet, there should be a comment telling you information about how this was generated. Can you please (1) open a bug, and (2) paste that header into the description? Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants