-
Notifications
You must be signed in to change notification settings - Fork 12
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
FT: Review feedback #46
Labels
Comments
Should make another pass at this guide and the provided implementation later with a fresh critical pair of eyes to tidy things up and make things clearer. Edit to be more consistent with the other guides too. |
Closed
Fixed the last feedback in #51
|
Open
close
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
General
What you'll learn
Repetitive and can be further simplified
Reconcile the use of 'inventory management application' and 'inventory service'
The implementation of the application and its services are provided for you. You can find the system service in the src/main/java/io/openliberty/guides/system directory, and you can find the inventory service in the src/main/java/io/openliberty/guides/inventory directory. These two services are RESTful web services, if you want to learn how to create one, see Creating a RESTful web service. This is unnecessary and can be removed?
Don't think we need a separate section on Why use MicroProfile Fault Tolerance either? We can introduce MP FT and focus on
@Fallback
?Try what you'll build
"finished fault tolerance implementation": This is weird. At least, it should be fault tolerant implementation? I don't think we need "fault tolerance" or "fault tolerant" there.
What the users are instructed to do here should be in terms of the services introduced in What you'll learn?
Because the system service is now in maintenance, you see the cached properties for this localhost. Should indicate fallback took place so cached data is used? Otherwise, how does the completed implementation relate to FT?
Enabling fault tolerance in the application
Use this feature to build fault-tolerant microservices with the MicroProfile Fault Tolerance API. This is inaccurate. The dependency is added so the API can be used in code. The use of "feature" here can confuse people. I will rephrase at the minimum. Should break the mention of the mpFaultTolerance-1.0 into a separate paragraph to avoid confusion.
I will express the introductory paragraphs in terms of the services provided and focus on what's relevant to FT/Fallback. Should simplify them too.
Building and running the application
Express in terms of the services that we have for this guide.
Point your browser to the http://localhost:9080/system/properties URL again, and notice that it is no longer available. This is difficult to parse. Should express in terms of the 'inventory' service and the 'system' service and how they relate to each other.
Testing the application
Because the application is a RESTful web service application, you can use JUnit and the REST Client API to write tests. Rephrase to be consistent with the other guides?
The test case then asserts whether the system properties that returned from the first request were more than the second. Explain why it should be more than the second one?
To see if the tests detect a failure, remove the change system property utility call on the FaultToleranceTest.java file. Rerun the Maven build. You see a test failure occur for the testFallbackForGet() test case. Remove this for now or rephrase since "change system property utility call" is difficult to understand.
Great work! You're done!
The text was updated successfully, but these errors were encountered: