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

Stage 1 Orchestrator RFC #1230

Merged
merged 20 commits into from
Mar 5, 2021
Merged

Stage 1 Orchestrator RFC #1230

merged 20 commits into from
Mar 5, 2021

Conversation

ferozsalam
Copy link
Contributor

@ferozsalam ferozsalam commented Jan 20, 2021

Moving the orchestrator RFC from stage 0 into stage 1

Preview of markdown proposal

@ebeahan ebeahan added the RFC label Jan 20, 2021
@cyrille-leclerc
Copy link
Contributor

FYI the Metricbeat is working on this topic in parallel, see elastic/beats#17467

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Thanks for moving this! Added some questions and suggestions.

Copy link
Member

@ebeahan ebeahan left a comment

Choose a reason for hiding this comment

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

Thanks for continuing to move this proposal forward, @ferozsalam! Excited to see all the discussions already taking place. 😃

Would you also update the target stage in the proposal: Stage: **1 (draft)**?

@ferozsalam
Copy link
Contributor Author

@ebeahan I'm slightly confused by the latest build failures, which I think are caused by something in the test framework that needs updating. The error refers to a Golang file that I haven't touched:

 golang.org/x/sys v0.0.0-20181217223516-dcdaa6325bcb h1:zzdd4xkMwu/GRxhSUJaCPh4/jil9kAbsU7AUmXboO+A=
 golang.org/x/sys v0.0.0-20181217223516-dcdaa6325bcb/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
 gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
git update-index --refresh
scripts/go.sum: needs update
make: *** [Makefile:25: check] Error 1
Error: Process completed with exit code 2.

This branch isn't missing any commits that are on master, and I've only touched the RFC Markdown, so I'm stumped. Any ideas?

@ebeahan
Copy link
Member

ebeahan commented Mar 1, 2021

@ebeahan I'm slightly confused by the latest build failures, which I think are caused by something in the test framework that needs updating.

@ferozsalam Thanks for the mention, it's being investigated. The pass/fail isn't applicable to any changes made here, so don't worry about the test failures.

Co-authored-by: Jaime Soriano Pastor <[email protected]>
kaiyan-sheng
kaiyan-sheng previously approved these changes Mar 5, 2021
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng left a comment

Choose a reason for hiding this comment

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

This looks good to me, thank you!!

ChrsMark
ChrsMark previously approved these changes Mar 5, 2021
jsoriano
jsoriano previously approved these changes Mar 5, 2021
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM

ebeahan
ebeahan previously approved these changes Mar 5, 2021
Copy link
Member

@ebeahan ebeahan left a comment

Choose a reason for hiding this comment

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

The collaboration here has been fantastic! Thanks to @ferozsalam for all the iterations and to @jsoriano @kaiyan-sheng @ChrsMark for all the feedback.

I noticed one possible minor edit, but otherwise, I think the proposal looks great.

Next steps:

  • It's been discussed out-of-band that @exekias will be acting as the sponsor here. I'll add @exekias to the People section when I update the advancement date. If this is incorrect, let me know.
  • The ECS team has found it useful to create a directory using the RFC number to capture examples and the proposed schema changes as standalone YML files (for use with --include). Just informational - no action required; I'll handle copy/pasting the YAML into that new file when I merge the proposal.
  • By advancing to stage 1, the orchestrator.* fields will be added to the experimental ECS schema. I'll also handle opening a separate PR for that implementation.
  • Once we merge this PR, I suggest creating the stage 2 PR shortly after even if the only change is updating the stage at the top of the document. It's great to already have the PR opened when thoughts or feedback come up.

@ebeahan ebeahan dismissed stale reviews from jsoriano, ChrsMark, kaiyan-sheng, and themself via f4352b0 March 5, 2021 19:41
@ebeahan ebeahan merged commit 9e63a5c into elastic:master Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants