This repository has been archived by the owner on Sep 17, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 42
feat: support using Beats CI snapshots #205
Merged
mdelapenya
merged 17 commits into
elastic:master
from
mdelapenya:204-use-beats-ci-snapshots
Jul 30, 2020
Merged
Changes from 6 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
f8b99c2
chore: use observability-ci Docker image for the elastic-agent
mdelapenya 37e6adc
feat: support downloading an object from a bucket on Google Cloud Sto…
mdelapenya 6e54a33
chore: add helper methods to read environment variables
mdelapenya 935a3bd
chore(ci): set the new env var for BeatsCI snapshots on CI
mdelapenya 9591228
fix: forgot to update reference
mdelapenya 220fd42
fix: set elastic-agent version for stand-alone mode
mdelapenya 3881a99
fix: make bool checks more strict
mdelapenya 06c2819
fix: variable initialisation
mdelapenya a888e7f
[CI] Docker login to access the internal registry
v1v 51ab097
fix: print indices for stand-alone ES queries
mdelapenya 6b0f132
chore: use observability-ci namespace everywhere
mdelapenya 6ea9afa
Merge branch 'master' into 204-use-beats-ci-snapshots
mdelapenya 4b10e64
chore: enhance param description
mdelapenya 9bffc6b
chore: group Jenkins params and variables by test suite
mdelapenya 5211896
feat: support consuming an elastic-agent from a PR, setting an env var
mdelapenya 618630e
chore: standardise variable names
mdelapenya 7038847
chore: do not use the neutral element for multiplication
mdelapenya File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kind of wonder if this should return something else here instead of
false
. (Perhapsnil
?) It seems like this behavior might introduce hard-to-handle error conditions, where a value is incorrectly interpreted as being truly false instead of an error in processing the lookup itself.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmmm, I see your point: so if the env var is not present, then do not consider it as false... In this particular use case, we are going to check for true, only. If true, then execute the proper code, otherwise use default. But it could be the case that we use this function again checking for false, and if the var is not present, it will satisfy the condition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think returning (bool, error) will do the trick, although we'll have to handle errors on the consume side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added your suggestion in 3881a99, including unit tests for verification.