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

Use cobra for webserver #1987

Merged
merged 1 commit into from Apr 12, 2019

Conversation

2 participants
@pjdufour-truss
Copy link
Contributor

pjdufour-truss commented Apr 9, 2019

Description

This PR uses cobra for the webserver. The 3 commands are now:

  • milmove help - shows help
  • milmove completion - prints bash completion script
  • milmove serve - runs MilMove server
  • milmove version - prints version information as json

To get the help for serve it is milmove serve -h. milmove serve is the command that actually runs the server.

Reviewer Notes

Is there anything you would like reviewers to give additional scrutiny?

  1. Could someone confirm the changes to .vscode/launch.json?

Setup

The following will add the bash completion scripts to your shell.

make server_build
bin/milmove completion > /usr/local/etc/bash_completion.d/milmove

Code Review Verification Steps

  • Code follows the guidelines for Logging
  • The requirements listed in
    Querying the Database Safely
    have been satisfied.
  • Any new migrations/schema changes:
    • Follow our guidelines for zero-downtime deploys (see Zero-Downtime Deploys)
    • Have been communicated to #dp3-engineering
    • Secure migrations have been tested using scripts/run-prod-migrations
  • There are no aXe warnings for UI.
  • This works in Supported Browsers (Chrome, Firefox, IE, Edge).
  • Any new client dependencies (Google Analytics, hosted libraries, CDNs, etc) have been:
  • Tested in the Experimental environment (for changes to containers, app startup, or connection to data stores)
  • User facing changes have been reviewed by design.
  • Request review from a member of a different team.
  • Have the Pivotal acceptance criteria been met for this change?

References

Screenshots

TBD


flag.String("build", "build", "the directory to serve static files from.")
flag.String("config-dir", "config", "The location of server config files")
flag.String("env", "development", "The environment to run in, which configures the database.")
flag.StringP("env", "e", "development", "The environment to run in, which configures the database.")

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Apr 9, 2019

Contributor

At some point we need to change this to db-env because it's not used for anything else and adds some confusion about the environment var, which really means prod/staging/experimental.

@@ -299,7 +300,7 @@ func initFlags(flag *pflag.FlagSet) {
flag.String("iws-rbs-host", "", "Hostname for the IWS RBS")

// DB Config
flag.String("db-name", "dev_db", "Database Name")
flag.StringP("db-name", "d", "dev_db", "Database Name")

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Apr 9, 2019

Contributor

I'd be ok with using the same psql short flags for all of these.

str, err := json.Marshal(map[string]interface{}{
"gitBranch": gitBranch,
"gitCommit": gitCommit,
})

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Apr 9, 2019

Contributor

👍 - this is an excellent use of the version command.

@chrisgilmerproj
Copy link
Contributor

chrisgilmerproj left a comment

You'll still need to update the Dockerfile's to get this to work. Otherwise I think this is good to go!

@codecov

This comment has been minimized.

Copy link

codecov bot commented Apr 9, 2019

Codecov Report

Merging #1987 into master will increase coverage by 0.1%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master    #1987     +/-   ##
=========================================
+ Coverage   60.35%   60.45%   +0.1%     
=========================================
  Files         192      193      +1     
  Lines       12495    12436     -59     
=========================================
- Hits         7541     7518     -23     
+ Misses       4066     4033     -33     
+ Partials      888      885      -3

@pjdufour-truss pjdufour-truss force-pushed the webserver_cobra branch 4 times, most recently from cc1d171 to d9319de Apr 9, 2019

@chrisgilmerproj
Copy link
Contributor

chrisgilmerproj left a comment

👍

@pjdufour-truss pjdufour-truss force-pushed the webserver_cobra branch 4 times, most recently from c48aea3 to dd6c363 Apr 11, 2019

@pjdufour-truss pjdufour-truss force-pushed the webserver_cobra branch from dd6c363 to d4fd4fb Apr 12, 2019

@pjdufour-truss pjdufour-truss merged commit 5d18568 into master Apr 12, 2019

19 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
ci/circleci: acceptance_tests_experimental Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_local Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_staging Your tests passed on CircleCI!
Details
ci/circleci: build_app Your tests passed on CircleCI!
Details
ci/circleci: build_migrations Your tests passed on CircleCI!
Details
ci/circleci: build_tools Your tests passed on CircleCI!
Details
ci/circleci: client_test Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_api Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_mymove Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_office Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_tsp Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_golang Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_yarn Your tests passed on CircleCI!
Details
ci/circleci: pre_test Your tests passed on CircleCI!
Details
ci/circleci: server_test Your tests passed on CircleCI!
Details
ci/circleci: server_test_coverage Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 111c9b7...d4fd4fb
Details
codecov/project/go 60.28% (+0.1%) compared to 111c9b7
Details

@pjdufour-truss pjdufour-truss deleted the webserver_cobra branch Apr 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.