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

Refactor Server #1809

Merged
merged 2 commits into from Mar 5, 2019

Conversation

2 participants
@pjdufour-truss
Copy link
Contributor

pjdufour-truss commented Mar 4, 2019

Description

This PR refactors the server package to support graceful shutdown (#1799). This PR thins the differential between our custom server and the http.Server by aligning variables names and adding a server.CreateServer(input *CreateServerInput) func. The server now only overrides (*http.Server).ListenAndServeTLS() and moves config to const variables when possible.

Reviewer Notes

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

Setup

None

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 bin/run-prod-migrations
  • There are no aXe warnings for UI.
  • This works in IE.
  • 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

N.A.

@chrisgilmerproj
Copy link
Contributor

chrisgilmerproj left a comment

Update the tests and then this is ready to go:)

@codecov

This comment has been minimized.

Copy link

codecov bot commented Mar 4, 2019

Codecov Report

Merging #1809 into master will decrease coverage by 0.13%.
The diff coverage is 81.25%.

@@            Coverage Diff            @@
##           master   #1809      +/-   ##
=========================================
- Coverage   49.34%   49.2%   -0.14%     
=========================================
  Files         427     423       -4     
  Lines       18384   18064     -320     
  Branches     1630    1621       -9     
=========================================
- Hits         9071    8889     -182     
+ Misses       8515    8396     -119     
+ Partials      798     779      -19
@chrisgilmerproj
Copy link
Contributor

chrisgilmerproj left a comment

🚀

@pjdufour-truss pjdufour-truss force-pushed the refactor_server branch from 90dae6a to 4e6a849 Mar 5, 2019

@pjdufour-truss pjdufour-truss merged commit 0a60bb0 into master Mar 5, 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: client_test_coverage 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 81.25% of diff hit (target 49.34%)
Details
codecov/project Absolute coverage decreased by -0.13% but relative coverage increased by +31.9% compared to b838bb3
Details

@pjdufour-truss pjdufour-truss deleted the refactor_server branch Mar 5, 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.