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.

Sign up for GitHub

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

Jump to bottom

decider: initial import of Orchestrator #6582

Merged
merged 6 commits into from
Aug 23, 2020
Merged

decider: initial import of Orchestrator #6582

merged 6 commits into from
Aug 23, 2020

Conversation

sougou
Copy link
Contributor

@sougou sougou commented Aug 18, 2020

This project is code named maestro.

This copies the core code base from Orchestrator so we can modify it to become a native vitess component. This is the initial copy, with go modules and linter fixes. Not all linter errors are fixed, but the Orchestrator builds. There's otherwise no change in functionality.

deepthi
deepthi approved these changes Aug 18, 2020
View reviewed changes
Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

To be clear, this continues to build orchestrator as a separate binary, and no orchestrator code is included in vitess binaries, correct?
In that case, LGTM

@shlomi-noach
Copy link
Contributor

shlomi-noach commented Aug 19, 2020 •
edited
Loading

@sougou : to review this PR, is b1366b7 a faithful copy of original orchestrator give or take gomod and refactoring of the three dependant libraries? If b1366b7 can be trusted, then I'll create a new branch, say initial-orchestrator-import and refactor this PR to merge into that branch, so we can see the differences. WDYT?

@shlomi-noach shlomi-noach mentioned this pull request Aug 19, 2020
Merged
@shlomi-noach
Copy link
Contributor

I created #6593, which pulls into initial-orchestrator-import branch. If we approve and merge #6593 (no risk in doing so) then we can edit this PR to merge into initial-orchestrator-import and the changes are more review-able.

harshit-gangal
harshit-gangal approved these changes Aug 21, 2020
View reviewed changes
Copy link
Member

@harshit-gangal harshit-gangal left a comment

Choose a reason for hiding this comment

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

If there is no change in functionality and only plain import of orch code, I will let it merge.

@sougou sougou changed the title maestro: initial import of Orchestrator decider: initial import of Orchestrator Aug 21, 2020
@sougou
orc: initial copy
a478fe1 
* go modules enabled
* go -> go/vt/orchestrator
* go/cmd -> go/cmd
* vendor-> external: golib, raft, zk
* imports fixed
* lint checks are failing

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
saving lint work
52d0524 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
more golint
4127891 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
orc: lint part 3
2b876f1 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
orc: copy web files
430a3df 
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
orc: delete failing raft tests
d09aac9 
There is currently no plan to use raft. So, we should eventually
delete this entire dependency.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@shlomi-noach
Copy link
Contributor

planetscale#68 is a simplification of this PR, and is more review-able.

@shlomi-noach
Copy link
Contributor

actually, planetscale#68 is just linter changes. What I'd like to see is a PR with the actual code changes on top of import+linter.

@sougou
Copy link
Contributor Author

sougou commented Aug 23, 2020

actually, planetscale#68 is just linter changes. What I'd like to see is a PR with the actual code changes on top of import+linter.

In that case, you'll probably find the oc2 branch interesting. I can merge this and create a new PR from oc2 to master.

@sougou sougou merged commit 77e8db2 into vitessio:master Aug 23, 2020
@sougou sougou mentioned this pull request Aug 23, 2020
Closed
@sougou sougou deleted the ss-oc1-initial-orc branch August 24, 2020 01:46
@askdba askdba added this to the v8.0 milestone Oct 5, 2020
@setassociative setassociative mentioned this pull request Mar 5, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deepthi deepthi deepthi approved these changes

@harshit-gangal harshit-gangal harshit-gangal approved these changes

@shlomi-noach shlomi-noach Awaiting requested review from shlomi-noach

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v8.0
Development

Successfully merging this pull request may close these issues.
5 participants
@sougou @shlomi-noach @deepthi @harshit-gangal @askdba