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

fix(docker): Not all Sentry commands work through CMD #18599

Merged
merged 1 commit into from May 4, 2020

Conversation

BYK
Copy link
Contributor

@BYK BYK commented May 4, 2020

Following up to #16417, which assumed all commands are listed in individual modules. This was not the case and this PR broke commands like docker run getsentry/sentry:latest export.

In this PR, we run sentry help and dump its output with some sed and awk magic to a file to check against. This approach is:

  1. Simple
  2. Serves as a simple smoke test for the image
  3. Is independent of the underlying CLI libarary (not that we'll change it, but who knows).

Following up to #16417, which assumed all commands are listed in individual modules. This was not the case and this PR broke commands like `docker run getsentry/sentry:latest export`.

In this PR, we run `sentry help` and dump its output with some `sed` and `awk` magic to a file to check against. This approach is:

1. Simple
2. Serves as a simple smoke test for the image
3. Is independent of the underlying CLI libarary (not that we'll change it, but who knows).
@BYK BYK requested a review from mattrobenolt May 4, 2020
@BYK BYK requested a review from a team as a code owner May 4, 2020
@BYK BYK merged commit 3e0d8bf into master May 4, 2020
7 checks passed
@BYK BYK deleted the byk/fix/docker-commands branch May 4, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Dec 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants