IBMStockTrader / trade-history

Microservice that keeps a detailed history of all stock trades
Apache License 2.0
1 stars 19 forks source link

Update for MongoDB connection #50

Closed Raunak-S closed 3 years ago

jwalcorn commented 3 years ago

great work! a few minor questions:

  1. why did the Maven dependency change from kafka-clients (2.1.0) to kafka_2.11? was it just to move from 2.1.0 to 2.11? or is this implying more jars would get packaged into the war's WEB-INF/lib, as we need more than just the client part of Kafka now? does mpRM need more dependencies than the raw Kafka client stuff I'd been using?
  2. I see a new truststore (trust.p12). did you copy it from one of my other microservices (I created one and copied it to all the others), or do you make a new one? just curious - I remember when I made mine I had to specify that they don't expire for like 30 years, so it doesn't go bad on us a year from now (our last ones did expire and one day stuff stopped working!)
  3. did you put a new cert (for Mongo? or Event Streams?) in the truststore? or are you getting that from the cert the operator is now passing in from the config map as an env var? I'd assumed the latter
  4. did you also create or modify the keystore (key.p12)? I don't see it in the PR, but I do see a new stanza referencing it in the server.xml
  5. where does the microprofile-config.xml end up in the Docker image? is it in the war file? or just a file copied into the Docker container? I didn't see a change in the Dockerfile
  6. not a big deal, but listing mpConfig-1.4 in the server.xml is redundant, because I already list the "umbrella" feature microProfile-3.3 in the server.xml. this actually causes it to include all of the core MP features, which includes mpConfig. note that mpRM is NOT a core feature, so it does have to be explicitly listed (as you did!)
  7. note that next week Liberty 21.0.0.3 should show up on DockerHub, which will have the GA of MP 4.0. so I'll update all the microservices to all say the new microProfile-4.0 then (along with updating the Maven dependency to MP 4.0 in each pom.xml).
  8. but we'll need to check with Emily if mpRM will work with MP 4.0; she had said it could be incompatible (for reasons I didn't fully understand), requiring them to release an updated mpRM (like an mpReactiveMessaging-1.0.1 or something) to work with microProfile-4.0
  9. Emily will be excited that Stock Trader will now show off mpRM too, for our MP 4.0 book!
Raunak-S commented 3 years ago

great work! a few minor questions:

  1. why did the Maven dependency change from kafka-clients (2.1.0) to kafka_2.11? was it just to move from 2.1.0 to 2.11? or is this implying more jars would get packaged into the war's WEB-INF/lib, as we need more than just the client part of Kafka now? does mpRM need more dependencies than the raw Kafka client stuff I'd been using?
  2. I see a new truststore (trust.p12). did you copy it from one of my other microservices (I created one and copied it to all the others), or do you make a new one? just curious - I remember when I made mine I had to specify that they don't expire for like 30 years, so it doesn't go bad on us a year from now (our last ones did expire and one day stuff stopped working!)
  3. did you put a new cert (for Mongo? or Event Streams?) in the truststore? or are you getting that from the cert the operator is now passing in from the config map as an env var? I'd assumed the latter
  4. did you also create or modify the keystore (key.p12)? I don't see it in the PR, but I do see a new stanza referencing it in the server.xml
  5. where does the microprofile-config.xml end up in the Docker image? is it in the war file? or just a file copied into the Docker container? I didn't see a change in the Dockerfile
  6. not a big deal, but listing mpConfig-1.4 in the server.xml is redundant, because I already list the "umbrella" feature microProfile-3.3 in the server.xml. this actually causes it to include all of the core MP features, which includes mpConfig. note that mpRM is NOT a core feature, so it does have to be explicitly listed (as you did!)
  7. note that next week Liberty 21.0.0.3 should show up on DockerHub, which will have the GA of MP 4.0. so I'll update all the microservices to all say the new microProfile-4.0 then (along with updating the Maven dependency to MP 4.0 in each pom.xml).
  8. but we'll need to check with Emily if mpRM will work with MP 4.0; she had said it could be incompatible (for reasons I didn't fully understand), requiring them to release an updated mpRM (like an mpReactiveMessaging-1.0.1 or something) to work with microProfile-4.0
  9. Emily will be excited that Stock Trader will now show off mpRM too, for our MP 4.0 book!

no worries. for the questions:

  1. I changed the dependency because the docs I read about kafka connections were using kafka-clients v2.1.0 but I don't think there's a difference in what mpRM requires. (i'm not even sure if the apache kafka dependency is even needed anymore since mpRM has its own kafka connector, so I might try removing the dependency altogether and see what happens) edit: kafka-clients v2.1.0 works
  2. I created a new truststore with keytool but I never specified the expiration. I can go back and copy over the trust.p12 from another microservice and put that in there
  3. i was only able to test the new cert stuff after creating the pull request so the trade-history trust.p12 includes the Mongo cert baked in. but since then I tested that it works with the redis cert in stock-quote so I can just skip baking in the Mongo cert when I copy over a truststore from another microservice
  4. this one just completely slipped my mind :sweat_smile: I planned to copy over the keystore from another microservice but probably got sidetracked and completely forget - i'll go it in there (i remember you said that we didn't need it because of basic auth right now but seems useful to keep just in case)
  5. just looked through the trade-history pod - looks the microprofile-config.properties is packaged in the war file and ends up in /opt/ol/wlp/usr/servers/defaultServer/apps/expanded/trade-history.war/WEB-INF/classes/META-INF when expanded