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

Change coordinator to query and add m3query service #817

Merged
merged 9 commits into from
Aug 2, 2018

Conversation

benraskin92
Copy link
Collaborator

No description provided.

@codecov
Copy link

codecov bot commented Aug 1, 2018

Codecov Report

Merging #817 into master will decrease coverage by 0.11%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #817      +/-   ##
==========================================
- Coverage   78.03%   77.92%   -0.12%     
==========================================
  Files         368      368              
  Lines       31798    31798              
==========================================
- Hits        24815    24779      -36     
- Misses       5309     5337      +28     
- Partials     1674     1682       +8
Flag Coverage Δ
#coordinator ?
#dbnode 81.37% <ø> (-0.12%) ⬇️
#m3ninx 72.7% <ø> (ø) ⬆️
#query 60.81% <ø> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b425df5...529807b. Read the comment docs.

@benraskin92 benraskin92 changed the title [DO NOT REVIEW] Change coordinator to query Change coordinator to query and add m3query service Aug 1, 2018
@@ -21,7 +21,7 @@ LABEL maintainer="The M3DB Authors <[email protected]>"
EXPOSE 7201/tcp 7203/tcp

COPY --from=builder /go/src/github.com/m3db/m3db/bin/m3coordinator /bin/
COPY --from=builder /go/src/github.com/m3db/m3db/src/coordinator/config/m3coordinator-local-etcd.yml /etc/m3coordinator/m3coordinator.yml
COPY --from=builder /go/src/github.com/m3db/m3db/src/query/config/m3coordinator-local-etcd.yml /etc/m3coordinator/m3coordinator.yml
Copy link
Collaborator

Choose a reason for hiding this comment

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

hm do you want to chance the directory name here too

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah maybe we should move these config files outside of src completely? It's weird we have some YAML files in these dirs floating around.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i guess we should figure out if we want to have an m3coordinator and an m3query dockerfile? we can discuss today once everyone is in

@@ -12,7 +12,7 @@ TARGETS=("dbnode" "coordinator" "m3ninx")
target_patterns() {
case $1 in
'dbnode') echo "^mode|github.com/m3db/m3db/src/dbnode|github.com/m3db/m3db/src/cmd/services/m3dbnode";;
'coordinator') echo "^mode|github.com/m3db/m3db/src/coordinator|github.com/m3db/m3db/src/cmd/services/m3coordinator";;
'coordinator') echo "^mode|github.com/m3db/m3db/src/query|github.com/m3db/m3db/src/cmd/services/m3coordinator";;
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm maybe rename this to 'query' too

Copy link
Collaborator

Choose a reason for hiding this comment

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

similarly for the .codecov.yml file and -F flags in the makefile

@@ -25,7 +25,7 @@ import (
_ "net/http/pprof" // pprof: for debug listen server if configured
"os"

"github.com/m3db/m3db/src/coordinator/services/m3coordinator/server"
"github.com/m3db/m3db/src/query/services/m3coordinator/server"
Copy link
Collaborator

Choose a reason for hiding this comment

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

The plan is to follow up and move the package being imported here to src/coordinator/server/... yeah?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes- and register the placement handlers there and remove them from src/github.com/m3db/m3db/src/query/services/m3coordinator/server (and also rename this to .../services/m3query/...

Copy link
Collaborator

@robskillington robskillington left a comment

Choose a reason for hiding this comment

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

LGTM

@benraskin92 benraskin92 merged commit c6505ba into master Aug 2, 2018
@benraskin92 benraskin92 deleted the braskin/add_m3query branch August 2, 2018 21:39
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