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

Request: BBL_STATE_DIR calculates full(er) path #25

Closed
evanfarrar opened this issue Aug 9, 2017 · 8 comments
Closed

Request: BBL_STATE_DIR calculates full(er) path #25

evanfarrar opened this issue Aug 9, 2017 · 8 comments
Labels

Comments

@evanfarrar
Copy link
Member

BBL is considering adding a BBL_STATE_DIR flag to mirror the functionality --state-dir flag. We actually tried shipping it, and then noticed in pipelines that this broke pipelines because of the process you follow, simplified like so:

cd $BBL_STATE_DIR
bbl up

Sure: we could pick a different name, or you could pick a different name, but I think in the long run it's a little more intuitive for people looking at the concourse task if all BBL_ are environment variables that BBL understands. For instance, I can check out my bbl-states repo, cd to that repo, set the vars I use in my concourse YML, and just bbl up. So my team has proposed this change as an example of a way that you can retain the name BBL_STATE_DIR and have a process that would be compatible with BBL before the change and after the change. Then long after the change you can elect at some time in the future to just cd bbl-states and allow BBL_STATE_DIR to be implicitly passed to bbl.

  local root_dir
  root_dir="${1}"
  export BBL_STATE_DIR=${root_dir}/bbl-state/${BBL_STATE_DIR}

...

+  mkdir -p "${BBL_STATE_DIR}"
+  pushd "${BBL_STATE_DIR}"
-  mkdir -p "bbl-state/${BBL_STATE_DIR}"
-  pushd "bbl-state/${BBL_STATE_DIR}"
  bbl up
@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/150113066

The labels on this github issue will be updated when the story is started.

@evanfarrar evanfarrar changed the title Request: allow Request: BBL_STATE_DIR calculates full(er) path Aug 9, 2017
@dsabeti
Copy link
Contributor

dsabeti commented Aug 10, 2017

It looks like the critical line here is:

  export BBL_STATE_DIR=${root_dir}/bbl-state/${BBL_STATE_DIR}

Essentially, our Concourse task would allow users to set their BBL_STATE_DIR to be relative to the bbl-state input, but we'd expand the relative path to the full path (based on the root_dir of the concourse build).

In that case, what does bbl expect BBL_STATE_DIR to look like -- a relative path or an absolute one? In the workflow you described, where a user copies the variables in their pipeline yaml, would bbl still be able to respect the relative path defined there?

@dsabeti
Copy link
Contributor

dsabeti commented Aug 18, 2017

@evanfarrar Hey, any updates on this?

@evanfarrar
Copy link
Member Author

Hey, sorry I missed this:

The BBL_STATE_DIR in bbl is fine being relative or absolute, we only expand it in the concourse task to incorporate the "bbl-state" parent directory scheme that cf-deployment-concourse follows. It could be expanded less fully in the task, if you prefer.

@dsabeti
Copy link
Contributor

dsabeti commented Aug 23, 2017

Hey @evanfarrar. This sounds reasonable. I've updated the story:

Title:
As a Concourse user, I want the bbl-up task to expand BBL_STATE_DIR to a full path so that, eventually, bbl can use the value natively.

Acceptance Criteria:

  • BBL_STATE_DIR should be converted into a full path early on in the script, a la BBL_STATE_DIR=${root_dir}/bbl-state/BBL_STATE_DIR
  • Subsequent uses of BBL_STATE_DIR should be not have the bbl-state namespace as in this line:
    pushd "bbl-state/${BBL_STATE_DIR}"

How does that look?

@evanfarrar
Copy link
Member Author

I have not answered this in forever but, that looks great.

@dsabeti
Copy link
Contributor

dsabeti commented Jan 25, 2018

Yo @evanfarrar, do we still need to do this since the release of bbl 5? If so, I'll prioritize it.

@dsabeti
Copy link
Contributor

dsabeti commented Mar 21, 2018

Hey, I'm going to close this out since I don't think we need to do this after all. Feel free to re-open it if you think this would still be valuable.

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

No branches or pull requests

3 participants