Added MySQL support back to ratings microservice #811

ErikWittern · 2017-09-14T20:02:23Z

Release note:

NONE

…expose an API

…oints to return JSON

@frankbu

…omment by @frankbu on pull request istio#693

istio-testing · 2017-09-14T20:02:31Z

Hi @ErikWittern. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

istio-merge-robot · 2017-09-14T20:03:14Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
We suggest the following additional approver: ldemailly

Assign the PR to them by writing /assign @ldemailly in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

OWNERS

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

rshriram

Thanks!

rshriram · 2017-09-14T20:34:27Z

don't merge yet

rshriram · 2017-09-14T20:53:30Z

/ok-to-test

ldemailly · 2017-09-14T21:02:30Z

samples/apps/bookinfo/src/mongodb/ratings_data.json

@@ -1,2 +1,2 @@
 {rating: 5}
-{rating: 6}


it's fine to have something different so you can visually tell which one you get

ldemailly · 2017-09-14T21:02:47Z

samples/apps/bookinfo/src/mysql/mysqldb-init.sql

@@ -10,4 +10,4 @@ CREATE TABLE `ratings` (
  PRIMARY KEY (`ReviewID`)
 );
 INSERT INTO ratings (ReviewID, Rating) VALUES (1, 5);
-INSERT INTO ratings (ReviewID, Rating) VALUES (2, 6);
+INSERT INTO ratings (ReviewID, Rating) VALUES (2, 4);


is 6 breaking the UI in some way ?

@ldemailly Yes, the UI only shows up to 5 "empty" stars. So, while the UI can display more than five "filled" stars, it would exceed the maximum a user would expect.

Yes, it looks pretty bad. May be use blue stars as next iteration?

ldemailly · 2017-09-14T21:02:57Z

samples/apps/bookinfo/src/ratings/ratings.js

 if (process.env.SERVICE_VERSION === 'v2') {
-  var MongoClient = require('mongodb').MongoClient
-  var url = process.env.MONGO_DB_URL
+  if (process.env.DB_TYPE === 'mysql') {


thanks!
is this mostly reverting to what it was before ?

yes. Reverting and cleaning up. We can now use API driven tests and no longer need html comparators

@ldemailly In this implementation, the DB_TYPE variable needs to be set to mysql for MySQL to be used. This is in line with changes @rshriram did here: 22014c8#diff-6a25b9bedba073dde47bfc3559d560a1

@frankbu

* revised source code of microservices so they communicate in JSON and expose an API * make API accessible via ingress controller * added swagger definition of new API * removed HTML status pages of microservices; aligned their health endpoints to return JSON * do not display anything when ratings data is not present to address comment by @frankbu on pull request #693 * added back support for MySQL into ratings microservice * fixed trailing line * fixed space * fixed ratings data in mongo db * fixed ratings data in mysql db Former-commit-id: 584ab1b

@frankbu

* revised source code of microservices so they communicate in JSON and expose an API * make API accessible via ingress controller * added swagger definition of new API * removed HTML status pages of microservices; aligned their health endpoints to return JSON * do not display anything when ratings data is not present to address comment by @frankbu on pull request istio#693 * added back support for MySQL into ratings microservice * fixed trailing line * fixed space * fixed ratings data in mongo db * fixed ratings data in mysql db Former-commit-id: 584ab1b

@frankbu

* revised source code of microservices so they communicate in JSON and expose an API * make API accessible via ingress controller * added swagger definition of new API * removed HTML status pages of microservices; aligned their health endpoints to return JSON * do not display anything when ratings data is not present to address comment by @frankbu on pull request #693 * added back support for MySQL into ratings microservice * fixed trailing line * fixed space * fixed ratings data in mongo db * fixed ratings data in mysql db Former-commit-id: 584ab1b

* Doc fixes. * Add high-level overview and diagrams for MCP (istio#805) * document MCP * add additional note on NACK behavior * add a note about ignoring stale nonces * Add envoy metrics service configuration to ProxyConfig (istio#803) * Add envoy metrics service configuration to ProxyConfig Similar to the statsd sink config, this change is in preparation for adding support for the envoy metrics service API. * Update proto.lock * Reorder metrics_service_address just after statsd_udp_address * Update comment in the authN API to match with 1.1 behavior (istio#808) * Rename metrics_service_address to clarify that it is for Envoy's API (istio#809) Since "metrics service" is so ambiguous, this change specifically denotes that the config is for Envoy's metrics service API. The comments also now link to detailed info about the metrics service API. * Generate separate HTML page per proto under networking (istio#811) * Generate separate pages for each networking proto Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * update Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * tweaks Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * Fix interdoc references * separate doc and go gen * change dash to underscore * fix * more fixes * makefile fix Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * remove bad target * Revert "change dash to underscore" This reverts commit 7fbced66f620fb60b0d5e18af916567e313a0109. * final update Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * Revert "Generate separate HTML page per proto under networking (istio#811)" This reverts commit 5f14eadd since it breaks the ability to generate reference docs in istio.io * Update to latest tools. * Revert "Update to latest tools." This reverts commit 231fadc5 which I accidentally pushed to the wrong place. * fix mcp update error diagram (istio#810) * update istio/tools (istio#815) * Revert "Revert "Generate separate HTML page per proto under networking (istio#811)"" (istio#818) * Minor updates for building docker images. (istio#817) * Add spell checking for docs. (istio#819) * Add a bunch of words to the custom dictionary. * Document exportTo restrictions (istio#820) * Doc fixes. (istio#821) * Proxy config: Add tls_certs_to_watch field (istio#824) * Proxy config: Add tls_certs_to_watch field * Address PR comment * Revert "Proxy config: Add tls_certs_to_watch field (istio#824)" (istio#825) This reverts commit e613e358ae8d4f331dd48904b0a2b176e11bf7f5. * Update gateway and sidecar hosts doc (istio#826) * Update gateway and sidecar hosts doc * regen * review comments * Update API docs (service entry, gateway, sidecar) (istio#831) * Update sidecar and gateway docs Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * gen docs Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * update service entry Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * updates Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * add configurable dns refresh rate to mesh for strict dns cluster, (istio#832) * add configurable dns refresh rate to mesh for strict dns cluster, issue istio#12181 * add lock file * Post-merge generation

* Doc fixes. * Add high-level overview and diagrams for MCP (istio#805) * document MCP * add additional note on NACK behavior * add a note about ignoring stale nonces * Add envoy metrics service configuration to ProxyConfig (istio#803) * Add envoy metrics service configuration to ProxyConfig Similar to the statsd sink config, this change is in preparation for adding support for the envoy metrics service API. * Update proto.lock * Reorder metrics_service_address just after statsd_udp_address * Update comment in the authN API to match with 1.1 behavior (istio#808) * Rename metrics_service_address to clarify that it is for Envoy's API (istio#809) Since "metrics service" is so ambiguous, this change specifically denotes that the config is for Envoy's metrics service API. The comments also now link to detailed info about the metrics service API. * Generate separate HTML page per proto under networking (istio#811) * Generate separate pages for each networking proto Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * update Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * tweaks Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * Fix interdoc references * separate doc and go gen * change dash to underscore * fix * more fixes * makefile fix Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * remove bad target * Revert "change dash to underscore" This reverts commit 7fbced66f620fb60b0d5e18af916567e313a0109. * final update Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * Revert "Generate separate HTML page per proto under networking (istio#811)" This reverts commit 5f14eadd since it breaks the ability to generate reference docs in istio.io * Update to latest tools. * Revert "Update to latest tools." This reverts commit 231fadc5 which I accidentally pushed to the wrong place. * fix mcp update error diagram (istio#810) * update istio/tools (istio#815) * Revert "Revert "Generate separate HTML page per proto under networking (istio#811)"" (istio#818) * Minor updates for building docker images. (istio#817) * Add spell checking for docs. (istio#819) * Add a bunch of words to the custom dictionary. * Document exportTo restrictions (istio#820) * Doc fixes. (istio#821) * Proxy config: Add tls_certs_to_watch field (istio#824) * Proxy config: Add tls_certs_to_watch field * Address PR comment * Revert "Proxy config: Add tls_certs_to_watch field (istio#824)" (istio#825) This reverts commit e613e358ae8d4f331dd48904b0a2b176e11bf7f5. * Update gateway and sidecar hosts doc (istio#826) * Update gateway and sidecar hosts doc * regen * review comments * Update API docs (service entry, gateway, sidecar) (istio#831) * Update sidecar and gateway docs Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * gen docs Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * update service entry Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * updates Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * add configurable dns refresh rate to mesh for strict dns cluster, (istio#832) * add configurable dns refresh rate to mesh for strict dns cluster, issue istio#12181 * add lock file * Hosts are not ignored in ServiceEntry for HTTPs (istio#833) * Hosts are not ignored in ServiceEntry for HTTPs * Clarify * Reword * Generate protos * Address code review comments * Fix typo * generate * make mixer config marshal deterministic (istio#846) Signed-off-by: Kuat Yessenov <kuat@google.com> * Fix some doc errors. (istio#854) This started out to just fix a bad link to unblock work on istio.io, but I ended up fixing a bunch of typos/incorrect format throughout. * Fix two more broken links. (istio#856) * Fix CRD examples for release-1.1 (istio#859) * fix MeshNetworks demo (istio#841) * fix MeshNetworks demo * generate * update * last minute doc fixes (istio#865) Signed-off-by: Shriram Rajagopalan <rshriram@gmail.com> * Clarify comment for server_name field in Mixer oop auth config (istio#800) * add mixerclient config to istio.io reference docs (istio#827) * Envoyfilter docs fix (istio#867) * Tweaks to get the mixer client config docs to be published. (istio#868) * Change where the Mixer client docs are published.

Erik Wittern and others added 16 commits September 7, 2017 11:55

revised source code of microservices so they communicate in JSON and …

70c0de2

…expose an API

make API accessible via ingress controller

4a392f2

added swagger definition of new API

e977f06

removed HTML status pages of microservices; aligned their health endp…

37cb9cd

…oints to return JSON

Merge remote-tracking branch 'upstream/master'

4e65fb2

Merge branch 'master' into master

d05dffc

Merge branch 'master' into master

89f1678

Merge remote-tracking branch 'upstream/master'

ed5c802

do not display anything when ratings data is not present to address c…

04ee5fa

…omment by @frankbu on pull request istio#693

Merge branch 'master' of https://github.com/ErikWittern/istio

a6d9c5d

Merge branch 'master' into master

faae06e

merge

b005cd4

Merge remote-tracking branch 'upstream/master'

90b036a

added back support for MySQL into ratings microservice

044487e

fixed trailing line

ff042ff

fixed space

14c88e5

istio-testing added the needs-ok-to-test label Sep 14, 2017

googlebot added the cla: yes label Sep 14, 2017

istio-merge-robot added the release-note-none label Sep 14, 2017

Erik Wittern added 2 commits September 14, 2017 16:13

fixed ratings data in mongo db

aad3d31

fixed ratings data in mysql db

260ecd8

rshriram approved these changes Sep 14, 2017

View reviewed changes

istio-testing removed the needs-ok-to-test label Sep 14, 2017

ldemailly self-requested a review September 14, 2017 21:01

ldemailly reviewed Sep 14, 2017

View reviewed changes

rshriram merged commit 584ab1b into istio:master Sep 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Added MySQL support back to ratings microservice #811

Added MySQL support back to ratings microservice #811

Uh oh!

ErikWittern commented Sep 14, 2017

Uh oh!

istio-testing commented Sep 14, 2017

Uh oh!

istio-merge-robot commented Sep 14, 2017

Uh oh!

rshriram left a comment

Uh oh!

rshriram commented Sep 14, 2017

Uh oh!

rshriram commented Sep 14, 2017

Uh oh!

ldemailly Sep 14, 2017

Uh oh!

ldemailly Sep 14, 2017

Uh oh!

ErikWittern Sep 14, 2017 •

edited

Loading

Uh oh!

rshriram Sep 14, 2017

Uh oh!

ldemailly Sep 14, 2017

Uh oh!

rshriram Sep 14, 2017

Uh oh!

ErikWittern Sep 14, 2017

Uh oh!

Uh oh!

Added MySQL support back to ratings microservice #811

Added MySQL support back to ratings microservice #811

Uh oh!

Conversation

ErikWittern commented Sep 14, 2017

Uh oh!

istio-testing commented Sep 14, 2017

Uh oh!

istio-merge-robot commented Sep 14, 2017

Uh oh!

rshriram left a comment

Choose a reason for hiding this comment

Uh oh!

rshriram commented Sep 14, 2017

Uh oh!

rshriram commented Sep 14, 2017

Uh oh!

ldemailly Sep 14, 2017

Choose a reason for hiding this comment

Uh oh!

ldemailly Sep 14, 2017

Choose a reason for hiding this comment

Uh oh!

ErikWittern Sep 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rshriram Sep 14, 2017

Choose a reason for hiding this comment

Uh oh!

ldemailly Sep 14, 2017

Choose a reason for hiding this comment

Uh oh!

rshriram Sep 14, 2017

Choose a reason for hiding this comment

Uh oh!

ErikWittern Sep 14, 2017

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ErikWittern Sep 14, 2017 •

edited

Loading