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

DT-598: Fixes #2722: ci.blt.yml not consistently applied. #3857

Merged
merged 1 commit into from
Sep 23, 2019

Conversation

danepowell
Copy link
Contributor

@danepowell danepowell commented Sep 19, 2019

Fixes #2722

Changes proposed

  • Add documentation about how yml properties are expanded
  • Fix ci environment not being detected when building artifact on Pipelines

Steps to replicate the issue

  1. Add a post-deploy-build command to ci.blt.yml, i.e.:
command-hooks:
  post-deploy-build:
    dir: /tmp/artifact
    command: 'npm install'
  1. Commit and start a pipelines job.

Previous (bad) behavior, before applying PR

The post-deploy-build command is not run

Expected behavior, after applying PR and re-running test steps

The post-deploy-build command is run

@@ -7,7 +7,7 @@ export PATH=${COMPOSER_BIN}:${PATH}
# Generate artifact in a separate directory.
export NEW_BUILD_DIR=/tmp/artifact
blt deploy:check-dirty --ansi --verbose
blt artifact:build -D deploy.dir=${NEW_BUILD_DIR} --ansi --verbose
blt artifact:build -D deploy.dir=${NEW_BUILD_DIR} --environment=ci --ansi --verbose
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 the second or third bug we've caught where this parameter was missing.

There are ways we could refactor this to avoid passing --environment=ci every time, such as setting BLT_ENV=ci as an environment variable once at the start of the script, or by incorporating EnvironmentDetector::isCiEnv() into the DefaultConfig environment detection step. They aren't without risk though, and at any rate are beyond the scope of this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

CI=true is present in most CI environments today. If not (looking at Jenkins, people can set it).

Why does blt have to act differently in CI vs on your machine ?

Copy link
Contributor Author

@danepowell danepowell Sep 23, 2019

Choose a reason for hiding this comment

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

Good point about the CI variable, we should key off of that. I created this followup issue: #3859

This ticket is itself a good example of why BLT needs to behave differently. System paths are different, available resources are different, you need different modules enabled, etc... In this particular case, the deploy directory is pretty special in Pipelines.

@danepowell danepowell added the Bug Something isn't working label Sep 19, 2019
@@ -7,7 +7,7 @@ export PATH=${COMPOSER_BIN}:${PATH}
# Generate artifact in a separate directory.
export NEW_BUILD_DIR=/tmp/artifact
blt deploy:check-dirty --ansi --verbose
blt artifact:build -D deploy.dir=${NEW_BUILD_DIR} --ansi --verbose
blt artifact:build -D deploy.dir=${NEW_BUILD_DIR} --environment=ci --ansi --verbose
Copy link
Contributor

Choose a reason for hiding this comment

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

CI=true is present in most CI environments today. If not (looking at Jenkins, people can set it).

Why does blt have to act differently in CI vs on your machine ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DT-598: Yaml placeholders are not correctly expanded in runtime parameters
3 participants