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

Allow for overriding pipelines and private variable settings in the constructor arguments #1

Closed
wants to merge 2 commits into from
Closed

Conversation

ebmeierj
Copy link
Contributor

Our pipeline files do not necessarily match the defaults. It is helpful to allow us to override these defaults when constructing the concourse object.

@flavorjones
Copy link
Owner

Hi, @ebmeierj! Thanks so much for submitting this. I'd like to ask two questions ...

The default behavior is to have an implicit name relationship between the ERB file and the final YML file. Without knowing more about your use case, I can't know for sure, but I'm curious if this is helpful in your case? For example, if I specify the ERB filename, would it be useful to have an intelligent default for the pipeline_filename? Or do you always want to specify both filenames?

Second, I've been planning on adding the ability to manage multiple pipeline files for a single project. Currently I try to use Concourse groups for this (see https://ci.nokogiri.org/teams/nokogiri-core/pipelines/nokogiri for an example) but this UX is suboptimal when looking at the recently-added top-level dashboard (see https://ci.nokogiri.org/). So the alternative I've been pondering is to specify multiple pipelines (one for master, one for PRs, one for allow-fail, etc.). Does this more directly match your use case? And if so, would you like to collaborate on that with me?

Thanks for reading, and thanks for submitting this PR!

@ebmeierj
Copy link
Contributor Author

ebmeierj commented Dec 4, 2018

Ah, yes - the ability to change the relationship between the ERB file and the final YML file was not part of my use case. It would actually simplify my instantiation if the 'final' filename is a derivative of the erb filename.

On a side note, I'm actually creating the 'final' file as a temp file for each 'set_pipeline' request so that I know that it is up to date. I'm not in love with that solution, but it's where I'm at for the time being.

We are currently experimenting with creating a separate 'PR' pipeline for each feature branch. The result of that is that we will only have one pipeline be relevant for the current feature at a time. If this strategy works for us, I don't think the need to specify multiple pipelines will be necessary for us. But the honest answer is that I'm not sure yet.

@flavorjones
Copy link
Owner

We are currently experimenting with creating a separate 'PR' pipeline for each feature branch.

Oh, interesting. Have you seen the work that the Concourse team has been doing around Spatial Resources to address this use case?

It would actually simplify my instantiation if the 'final' filename is a derivative of the erb filename.

Are you happy with the current relationship of #{project_name}.yml#{project_name}.final.yml? Would you prefer a different pattern?

@ebmeierj
Copy link
Contributor Author

ebmeierj commented Dec 5, 2018

I knew there was an Epic out there to address it - but I need this to be useful for us now and don't have any real expectations on when Spatial Resources will be available. It would definitely solve my problem more elegantly, though.

I don't have any strong feelings on the current filename relationship. Personally, I'm creating the "final" file as a Tempfile in the set_pipeline task and removing it after the task completes. That way I don't have to remember to run generate any time I make changes to the base pipeline erb file. So I'm not really using the 'final' filename.

@ebmeierj
Copy link
Contributor Author

ebmeierj commented Dec 6, 2018

Hm... it seems I missed that generate was a dependency of set - so I now realize the gem was generating the "final" file every time the pipeline was set as well. In that case, maybe #{project_name}.generated.yml is a bit more clear? Personally I still like the idea of using a temporary file that gets cleaned up after the program exits... but I can also see value from having the file stick around so that it is easier to examine for errors and such.

@flavorjones flavorjones self-assigned this Dec 29, 2018
flavorjones added a commit that referenced this pull request Jan 18, 2019
flavorjones added a commit that referenced this pull request Jan 18, 2019
* fly_target
* pipeline_erb_filename
* secrets_filenam

along with additional test coverage, README and CHANGELOG updates.

related to #1
@flavorjones
Copy link
Owner

I ended up implementing this slightly differently than in this PR, but I've given you credit in the CHANGELOG. Will be in the next release. Please see README for how to use the new parameters.

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

Successfully merging this pull request may close these issues.

2 participants