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

FT: Review feedback #46

Closed
16 tasks done
yeekangc opened this issue Mar 16, 2018 · 4 comments
Closed
16 tasks done

FT: Review feedback #46

yeekangc opened this issue Mar 16, 2018 · 4 comments
Assignees
Labels

Comments

@yeekangc
Copy link
Member

yeekangc commented Mar 16, 2018

General

  • Use "services" over "application" where appropriate to be consistent with other guides?

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!

  • You have just completed building and testing a microservice application with MicroProfile Fault Tolerance and Open Liberty. We have a microservice application now?
@yeekangc
Copy link
Member Author

yeekangc commented Mar 16, 2018

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.
This can be a new issue.

@evelinec
Copy link
Contributor

evelinec commented Mar 28, 2018

Fixed the last feedback in #51

  • 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.

@evelinec evelinec changed the title Review feedback FT: Review feedback Apr 12, 2018
@yeekangc
Copy link
Member Author

yeekangc commented Dec 10, 2019

Expect feedback to be appropriately addressed and PRs merged.

Provide adequate context for #52 and #53 if they will be handled separately.

@gkwan-ibm
Copy link
Member

close

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants