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

Add logging statements to the shim #43

Merged
merged 2 commits into from
Apr 22, 2024

Conversation

schneems
Copy link
Contributor

@schneems schneems commented Jun 6, 2022

Right now then code in the shim fails it looks like the error is inside of the buildpack itself. For example this error:

[builder] -----> Detecting rails configuration
[builder] exit status 127
[builder] ERROR: failed to build: exit status 3
ERROR: failed to build: executing lifecycle. This may be the result of using an untrusted builder: failed with status code: 51

Appears to indicate that the problem happened in bin/compile when in fact that was successful and the real problem didn't happen until bin/release. By logging our intentions to the output it makes it clearer to understand where failures are happening if/when they do happen.

@schneems schneems marked this pull request as ready for review June 6, 2022 19:54
@heroku heroku bot temporarily deployed to cnb-shim-schneems-tell--h5hbi1 June 6, 2022 19:54 Inactive
@schneems schneems enabled auto-merge (squash) June 6, 2022 19:55
@schneems schneems force-pushed the schneems/tell-me-what-ur-doing-please branch from 6043ff3 to 4572c44 Compare June 7, 2022 16:21
@heroku heroku bot temporarily deployed to cnb-shim-schneems-tell--h5hbi1 June 7, 2022 16:22 Inactive
@heroku heroku bot temporarily deployed to cnb-shim-schneems-tell--aaqgaf October 13, 2022 08:45 Inactive
Right now then code in the shim fails it looks like the error is inside of the buildpack itself. For example this error:

```
[builder] -----> Detecting rails configuration
[builder] exit status 127
[builder] ERROR: failed to build: exit status 3
ERROR: failed to build: executing lifecycle. This may be the result of using an untrusted builder: failed with status code: 51
```

Appears to indicate that the problem happened in `bin/compile` when in fact that was successful and the real problem didn't happen until `bin/release`. By logging our intentions to the output it makes it clearer to understand where failures are happening if/when they do happen.
@edmorley edmorley force-pushed the schneems/tell-me-what-ur-doing-please branch from 4572c44 to 3e1b6a9 Compare April 19, 2024 16:21
@schneems schneems requested a review from a team as a code owner April 19, 2024 16:21
* Prefix all output with `CNB Shim:` since otherwise it's not
  clear whether its the buildpack or the shim's output.
* Move the output for optional steps inside the step's conditional
* Adjust some of the step wordings
@heroku heroku bot temporarily deployed to cnb-shim-schneems-tell--3uftbg April 19, 2024 16:59 Inactive
@edmorley
Copy link
Member

cnb-shim is EOL so on the most part we won't be spending any more time on it. However, since this is a quick win, lets merge this.

I rebased this on main, resolved the conflicts, and made a couple of tweaks:

  • Prefixed all output with CNB Shim: since otherwise it's not clear whether its the buildpack or the shim's output.
  • Moved the output for optional steps inside the step's conditional
  • Adjusted some of the step wordings

For the changes, see:
50e0183

I triggered a builder CI run using the Review App for this PR - tests passed:
https://github.com/heroku/cnb-builder-images/actions/runs/8757068659

And the new output can be seen in eg:
https://github.com/heroku/cnb-builder-images/actions/runs/8757068659/job/24035023408#step:6:96

@edmorley edmorley disabled auto-merge April 19, 2024 17:06
@edmorley edmorley enabled auto-merge (squash) April 19, 2024 17:06
@edmorley edmorley merged commit 0e8e93a into main Apr 22, 2024
1 check passed
@edmorley edmorley deleted the schneems/tell-me-what-ur-doing-please branch April 22, 2024 07:45
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.

2 participants