nulecule icon indicating copy to clipboard operation
nulecule copied to clipboard

Implement param pointers to Nulecule spec.

Open cdrage opened this issue 10 years ago • 18 comments

This implements XPathing as per the Atomic App PR https://github.com/projectatomic/atomicapp/pull/366.

Fixes issue #70.

I'm not too good at modfying / changing spec information, so will need some input from @bexelbie and @aweiteka

cdrage avatar Nov 27 '15 15:11 cdrage

can you add a little why and what to the XPath chapter. Same for "XPathing", it seems to be called JSON patching, as in the RFC you mentioned.

goern avatar Dec 01 '15 15:12 goern

Per atomicapp pr#366 shouldn't this be part of the artifacts object?

aweiteka avatar Dec 02 '15 14:12 aweiteka

@aweiteka @goern

Yes, it's suppose to be part of the artifacts object but I was unable to find a clean example to indicate it (the spec uses different definitions than our atomic app implementation).

~~Could one of you please take-over this issue and possibly create another PR to merge in? I'm not too good at modifying/improving specifications.~~ let me update and try again :)

cdrage avatar Dec 02 '15 14:12 cdrage

ping @aweiteka @goern updated again

cdrage avatar Dec 17 '15 16:12 cdrage

timeout... pinging again @goern @aweiteka

and @vpavlin

cdrage avatar Dec 21 '15 16:12 cdrage

@cdrage I'm confused how this relates to this[1]. It seems like a duplication. I don't want to ask more of you so I think at this point I would like to take a pass at this. Would you mind?

[1] https://github.com/projectatomic/nulecule/commit/7a83d58c7ff34dbc00c94be252921d384e6092b0

aweiteka avatar Dec 22 '15 13:12 aweiteka

@aweiteka That would be my mistake, seems I had merged xpathing during the 0.3.0 release by mistake. I've since reverted that.

Feel free to check this PR again :)

cdrage avatar Dec 22 '15 16:12 cdrage

param:
  - /spec/containers/0/param

I still can't reconcile this with atomicapp implementation https://github.com/projectatomic/atomicapp/pull/366 . I don't think it belongs in constraints. I also believe it is better described in artifacts, not params definition.

aweiteka avatar Dec 22 '15 19:12 aweiteka

@cdrage I submitted PR to update your branch.

aweiteka avatar Dec 22 '15 19:12 aweiteka

ping @aweiteka @bexelbie can we merge this?

cdrage avatar Jan 12 '16 16:01 cdrage

timeout, pinging again @aweiteka @bexelbie

cdrage avatar Feb 11 '16 18:02 cdrage

LGTM

@vpavlin ?

goern avatar Feb 11 '16 19:02 goern

I agree with @aweiteka that this belongs in the artifacts section or at least not in constraints.

If atomic app varies from the spec we should rationalize the two. Perhaps we need a working spec call?

bexelbie avatar Feb 12 '16 09:02 bexelbie

@bexelbie @goern @dustymabe

This is badddd documentation in my regards, this should have never been committed. I will update this.

I also vote to move more towards yaml instead since it gives a lot more clarity in regards to the spec (not only the xpathing examples).

This is what xpathing looks like within Atomic App and thus within artifacts:

artifacts:
   kubernetes:
       - resource: artifacts/kubernetes/template.json
         params:
            foo:
               - /spec/template/metadata/foo
            bar:
               - /spec/template/metadata/bar
      - resource: artifacts/kubernetes/extra.json

cdrage avatar Mar 14 '16 03:03 cdrage

@cdrage.. can we get a TL;DR for this? I'm not sure I follow everything.

dustymabe avatar Mar 14 '16 23:03 dustymabe

ping @cdrage

dustymabe avatar Apr 01 '16 14:04 dustymabe

TLDR:

If you wish to change a value within a json/yaml file, you can specify the value and position in the file to replace, ex:

            param1:
                - /spec/containers/0/ports/0/hostPort
                - /spec/containers/0/ports/0/hostPort2

cdrage avatar Apr 07 '16 15:04 cdrage

@goern @aweiteka

Once #209 is merged, I can add this to the spec as it'll be easier to understand from a usability perspective on how to use this.

cdrage avatar Jun 06 '16 12:06 cdrage