OpenLiberty / guide-mongodb-intro

A guide on how to persist data in your microservices to MongoDB, a document-oriented NoSQL database.
https://openliberty.io/guides/mongodb-intro.html
Other
3 stars 4 forks source link

SME Review #35

Closed jimmy1wu closed 4 years ago

jimmy1wu commented 4 years ago

SME Review:

Review of the guide by a Subject Matter Expert (SME) for technical accuracy and content.

mswatosh commented 4 years ago

I built the docker file and started the container, then copied the truststore and started the server. Whenever I try to interact with the application I just get SRVE0190E: File not found: /api/crew/

The application doesn't work and the tests fail. Is this a known issue?

What you’ll learn

I'm worried the setup is too complicated, and that it might scare away users looking to quickly setup mongoldb with Open Liberty. The docker file is pretty complicated, could we just provide a certificate instead of generating it to simplify?

Potentially instead of making changes to the mongo docker image, just use the command line args: docker run -d -v /path/to/pem/:/etc/ssl/ mongo:3.6 --tlsMode requireTLS --tlsCertificateKeyFile /etc/ssl/mongodb.pem

Starting the Open Liberty server in development mode & Configuring with MicroProfile Config

Read

CrewService.java should have the methods organized in the same order as the guide, so the autoscrolling doesn't jump around so much.

Running the micro service

The test section goes into detail on testing in general, is there a guide we could point to instead, so the section could be more relevant to the mongodb tests?

I would recommend against the underscore notation in the test class. It's uncommon in Java and inconsistent with the rest of the code base.

The package for the test class should start io.openliberty not it.io.openliberty.

jimmy1wu commented 4 years ago

I built the docker file and started the container, then copied the truststore and started the server. Whenever I try to interact with the application I just get SRVE0190E: File not found: /api/crew/

We did not run into that issue before. Did this issue only occur when you were building the application from the start folder? or did the application in the finish folder fail for you as well?

@mswatosh

jimmy1wu commented 4 years ago

The docker file is pretty complicated, could we just provide a certificate instead of generating it to simplify?

Not sure about this one. Since the certs may expire and I'm not sure if we should upload private keys on github even if for demo

One part says command line and the other says shell session, this should be consistent

Determining which one we should use now. https://github.com/OpenLiberty/guides-common/issues/455

The package for the test class should start io.openliberty not it.io.openliberty.

The guides use it.io.openliberty for the test package.

Rest of the points have been addressed. Also see above comment. @mswatosh

mswatosh commented 4 years ago

Sorry, I haven't had a chance to investigate the issue. It was with the finish folder, I didn't try the start folder.

I would think as long as the certs were set to expire in 100 years it would be fine.

jimmy1wu commented 4 years ago

@mswatosh After discussing with YK, we decided to continue to generate the certs instead of providing them to maintain consistency with other guides.

Also, do you know when you will have time to look into the issue you were having?

mswatosh commented 4 years ago

This is approved