Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Project Template #18

Closed
wants to merge 4 commits into from
Closed

Project Template #18

wants to merge 4 commits into from

Conversation

mfojtik
Copy link
Contributor

@mfojtik mfojtik commented Aug 14, 2014

This is an WIP of the Project Template work.
The code below define the Project structs and implement parameters substitution for the JSON.

Changelog

  • Make getting Parameter more effective by converting the Parameters to Hash
  • The PValue#Substitute function now modifies the value of itself
  • Added ServiceLinks to SubstituteEnvValues
  • Added rand.Seed to ProcessParameters
  • Added godocs
  • Added ServiceLinks resolving in terms that environment variables exported by the ServiceLink are copied into target service.

@mfojtik mfojtik mentioned this pull request Aug 14, 2014
// s := generate.Template("[GET:http://example.com/new]")
// // s: <body from the GET request>
func (p *Template) ProcessParameters() {
rand.Seed(p.RandomSeed)
Copy link
Contributor

Choose a reason for hiding this comment

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

The template should have a *rand.Rand that is initialized at creation time. Methods should never change the seed

Copy link
Contributor

Choose a reason for hiding this comment

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

How are you going to handle passed parameters? What if I want to pass a parameter that is listed as generate? Needs to be a way to do that - remember that this is a transition process - so either the p is the process, or the method call is the process and the method call would need to take a thing to transform.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All parameters in parameters definition in JSON are used automatically, so you don't need to pass a list of them as an argument to transformation.
If the JSON defines parameter that has generate then the value is generated when this method is invoked for the first time.

Copy link
Contributor

Choose a reason for hiding this comment

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

So I think template transformations should create a new object just as a general cleanliness issue. You're loading a template and transforming it - it should be clear what is user input (override parameters) and what is part of the template (defaults / generation). When you invoke the transformation, those overrides should be explicitly passed, checked, then resolved. I just don't think process is a method on templates, it's a method on a transformation object

@mfojtik
Copy link
Contributor Author

mfojtik commented Aug 15, 2014

@smarterclayton PR updated, rand is now passed as a reference, so you don't need to initialize it globally, the helpers are not in their own file and I nuked httpGet and randomInt methods....
See the question about parameter transformation and this PR:

#21

Once we agree on the JSON template example, I will update the structs in this PR and processing of env vars.

@mfojtik
Copy link
Contributor Author

mfojtik commented Aug 19, 2014

@smarterclayton updated, I added ExampleTemplate_Transform method.

@smarterclayton any additional comments/review?

@mfojtik mfojtik changed the title [WIP] Project Template Project Template Aug 19, 2014
//
// s := generate.Template("[GET:http://example.com/new]")
// // s: <body from the GET request>
func (p *Template) ProcessParameters(customParams []Parameter) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In the API model, no methods can be defined on API model objects (they are DAO or value objects). So you need to create a templateProcessor object or something similar to handle taking a template and transforming it into a new template

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just processing the parameters, iow. generate will produce parameter 'value'. I do agree that the 'Transform' method should be probably defined on templateProcessor which should be most likely interfaced OR as a non-struct method (TransformTemplate(t *Template))

Copy link
Contributor

Choose a reason for hiding this comment

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

Again, this is answering my later question in pull/18. :)

@mfojtik
Copy link
Contributor Author

mfojtik commented Aug 20, 2014

@smarterclayton I revamped this PR into a new PR: #27

The new PR use the latest JSON schema (updating this PR was not so hard as I thought, so no reason to postpone it for the next sprint I think).

I'm going to close this PR, is it OK?

@mfojtik mfojtik closed this Aug 20, 2014
ironcladlou referenced this pull request in ironcladlou/origin Sep 29, 2014
soltysh referenced this pull request in soltysh/origin Nov 20, 2014
More rebase fixes to Template and Config
danwinship pushed a commit to danwinship/origin that referenced this pull request Jun 24, 2016
Use library function to get hostname.
sttts referenced this pull request in sttts/origin Dec 16, 2016
jpeeler pushed a commit to jpeeler/origin that referenced this pull request Feb 1, 2018
openshift-merge-robot added a commit that referenced this pull request Feb 7, 2018
Automatic merge from submit-queue.

CLI: add support for deployments in oc status

@smarterclayton this is a long overdue...

current state:
```
$ oc status
In project My Project (myproject) on server https://127.0.0.1:8443

svc/ruby-deploy - 172.30.174.234:8080
  deployment/ruby-deploy deploys istag/ruby-deploy:latest <-
    bc/ruby-deploy source builds https://github.com/openshift/ruby-ex.git on istag/ruby-22-centos7:latest
      build #1 failed 5 hours ago - bbb6701: Merge pull request #18 from durandom/master (Ben Parees <[email protected]>)
    deployment #2 running for 4 hours - 0/1 pods (warning: 53 restarts)
    deployment #1 deployed 5 hours ago
```

TODO:

- [x] Add rollouts similar to deployment configs
- [x] Fix unit tests / Add unit tests
- [x] Deal with HPA
@mfojtik mfojtik deleted the project branch September 5, 2018 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants