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

Change defaults that depending on the working dir #128

Merged
merged 2 commits into from
Aug 5, 2020

Conversation

ekcasey
Copy link
Member

@ekcasey ekcasey commented Jul 31, 2020

This has been a frequent request of the lifecycle. Default file paths relative to the working dir are confusing because we provide no guidance on what the working directory should be when the platform executes the lifecycle. Making defaults relative to layers is better because, when phases are executed in separate containers these files should exist on a volume so they may be passed between phases. Right now if a user changes the layers dir they must also change the path of group.toml, plan.toml, etc so that they files are passed between phases. This is annoying. If the defaults are relative to , changing the layers dir "just works" without requiring a variety of other flags.

Signed-off-by: Emily Casey ecasey@vmware.com

This has been a frequent request of the lifecycle. Default file paths relative to the working dir are confusing because we provide no guidance on what the working directory should be when the platform executes the lifecycle. Making defaults relative to layers is better because, when phases are executed in separate containers these files should exist on a volume so they may be passed between phases. Right now if a user changes the layers dir they must also change the path of group.toml, plan.toml, etc so that they files are passed between phases. This is annoying. If the defaults are relative to <layers>, changing the layers dir "just works" without requiring a variety of other flags.

Signed-off-by: Emily Casey <ecasey@vmware.com>
@ekcasey ekcasey added this to the Platform 0.5 milestone Jul 31, 2020
@ekcasey ekcasey requested a review from a team as a code owner July 31, 2020 15:29
| `<group>` | `CNB_GROUP_PATH` | `<layers>/group.toml` | Path to output group definition
| `<layers>` | `CNB_LAYERS_DIR` | `/layers` | Path to layers directory
| `<log-level>` | `CNB_LOG_LEVEL` | `info` | Log Level
| `<order>` | `CNB_ORDER_PATH` | `/cnb/order.toml` | Path to order definition (see [`order.toml`](#ordertoml-toml))
Copy link
Member Author

Choose a reason for hiding this comment

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

This one was just a mistake in the platform rewrite.

@nebhale nebhale requested a review from a team August 3, 2020 15:06
@jkutner
Copy link
Member

jkutner commented Aug 3, 2020

thanks! i ran into this just last week.

@nebhale nebhale requested a review from a team August 3, 2020 22:36
@hone
Copy link
Member

hone commented Aug 5, 2020

Yeah, I also found this annoying. I assume for the most part this won't break any existing users using lifecycle. Mostly would be people doing one off / tty of lifecycle?

@nebhale nebhale merged commit 2b87577 into platform/0.5 Aug 5, 2020
@nebhale nebhale deleted the default-paths-relative-to-layers branch August 5, 2020 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants