-
Notifications
You must be signed in to change notification settings - Fork 127
Fix tests around pipelines with custom dataset #1437
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
Conversation
The dataset value in the manifest.yml file can be overwritten to allow dataset names which are not `{package}.{dirName}`. One example of this can be found here: elastic/integrations#7670 This is already in use today and is used by some packages. Fleet installs all the assets as expected.
When working on elastic/integrations#7670, CI is failing. The reason is that the pipeline check did not take the custom dataset config into account. The reason why this didn't show up earlier is because the check for the template already has a similar condition in place. Likely all previous packages that used this feature did only have templates and no pipelines.
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.
LGTM
|
I'm wondering if the failure that CI hit is in any way related? The following can be seen before in the logs which makes me suspicious: What does elastic-package exactly test around aws.ec2 metrics? |
|
I did rerun the job and seems it was just flaky. |
|
This PR currently blocks elastic/integrations#7670 @jsoriano As soon as this PR is in, will CI in elastic/integrations#7670 pick up the new version or will we need to first release a new version? |
It's needed a new elastic-package release in order to be used by integrations. Once created the release, dependabot creates the required PR. |
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.
LGTM
Could you update the test package awsfirehouse (test/packages/parallel/awsfirehose) to use that custom dataset as in the integrations PR ? In that way, this change could also be tested here in elastic-package @ruflin
|
@mrodm Good idea, I'll update it. |
💚 Build Succeeded
History
cc @ruflin |
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.
Thanks!
The dataset value in the manifest.yml file can be overwritten to allow dataset names which are not `{package}.{dirName}`. One example of this can be found here: elastic/integrations#7670 This is already in use today and is used by some packages. Fleet installs all the assets as expected.
When working on elastic/integrations#7670, CI is failing. The reason is that the pipeline check did not take the custom dataset config into account. The reason why this didn't show up earlier is because the check for the template already has a similar condition in place. Likely all previous packages that used this feature did only have templates and no pipelines.
The dataset value in the manifest.yml file can be overwritten to allow dataset names which are not
{package}.{dirName}. One example of this can be found here: elastic/integrations#7670 This is already in use today and is used by some packages. Fleet installs all the assets as expected.When working on elastic/integrations#7670, CI is failing. The reason is that the pipeline check did not take the custom dataset config into account. The reason why this didn't show up earlier is because the check for the template already has a similar condition in place. Likely all previous packages that used this feature did only have templates and no pipelines.