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

Updating package-spec dependency #260

Merged
merged 12 commits into from
Feb 25, 2021

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Feb 23, 2021

This PR updates the dependency on the package-spec to bring in this change: elastic/package-spec#139.

Related: elastic/package-spec#106

@elasticmachine
Copy link
Collaborator

elasticmachine commented Feb 23, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #260 updated

  • Start Time: 2021-02-25T17:49:15.131+0000

  • Duration: 18 min 21 sec

  • Commit: ba99682

Test stats 🧪

Test Results
Failed 0
Passed 292
Skipped 1
Total 293

Trends 🧪

Image of Build Times

Image of Tests

@ycombinator
Copy link
Contributor Author

ycombinator commented Feb 23, 2021

CI is failing with this error:

[2021-02-23T00:59:28.131Z] ERROR: for elastic-agent_is_ready  Container "c1071590ccd8" is unhealthy.

[2021-02-23T00:59:28.131Z] Encountered errors while bringing up the project.

[2021-02-23T00:59:28.131Z] Error: booting up the stack failed: running docker-compose failed: running command failed: running Docker Compose up command failed: exit status 1

I believe this is due to recent changes in elastic-agent behavior which are going to be fixed in a new Docker image shortly. So we will simply retry CI again in a day or so.

@ycombinator ycombinator requested a review from mtojek February 23, 2021 01:03
@mtojek
Copy link
Contributor

mtojek commented Feb 23, 2021

/test

Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

The change looks good to me, but please wait for the green status.

@mtojek
Copy link
Contributor

mtojek commented Feb 23, 2021

Blocked by: elastic/beats#24163

@mtojek
Copy link
Contributor

mtojek commented Feb 24, 2021

@ycombinator I think it failed due to some issues around event.ingested.

@ycombinator ycombinator force-pushed the pipeline-test-configs-yml branch from 1c5663a to 14f0639 Compare February 24, 2021 17:32
@ycombinator ycombinator requested a review from mtojek February 24, 2021 20:24
@ycombinator
Copy link
Contributor Author

@mtojek I ended up making some more code changes to this PR even after applying the changes from #263. So I have re-requested your review. Thanks!

Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

Just a loose thought:

Personally I think that this is an overkill here (switch from JSON to YAML), because eventually we'll end up with correcting these files mostly on our own :)

IMHO you could go without deprecation period, but of course it's clearer.

Apart from this, LGTM!

@ycombinator
Copy link
Contributor Author

Personally I think that this is an overkill here (switch from JSON to YAML), because eventually we'll end up with correcting these files mostly on our own :)

IMHO you could go without deprecation period, but of course it's clearer.

Yeah, I agree about this when it comes to packages in integrations. The deprecation period is more for packages that are being developed outside of the integrations repo that we don't really know about. I doubt any of them are using pipeline tests but, just in case they are, better to be safe!

@ycombinator ycombinator merged commit 24a47f8 into elastic:master Feb 25, 2021
@ycombinator ycombinator deleted the pipeline-test-configs-yml branch February 25, 2021 18:11
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.

3 participants