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

Add PHP quote service #345

Merged

Conversation

julianocosta89
Copy link
Member

@julianocosta89 julianocosta89 commented Aug 29, 2022

Towards #57.

Changes

Adding PHP service - QuoteService, which will receive requests from ShippingService, calculate the shipping costs and return the quote to Shipping.

For significant contributions please make sure you have completed the following items:

@julianocosta89
Copy link
Member Author

@bobstrecansky could you take a look?
Any clean up suggestions are more than welcome.

@julianocosta89

This comment was marked as outdated.

@julianocosta89 julianocosta89 self-assigned this Aug 29, 2022
@julianocosta89 julianocosta89 added the in-progress This issue is actively being worked on label Aug 29, 2022
@bobstrecansky
Copy link

@brettmc / @tidal for review, as I'm OoO this week. At face value this looks good to me, but I'm also reading on a tiny screen 😎

Copy link
Contributor

@brettmc brettmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks pretty good to me. it's based on one of our demos and there are a couple of things unused that could be removed.

src/quoteservice/Dockerfile Outdated Show resolved Hide resolved
src/quoteservice/app/routes.php Show resolved Hide resolved
src/quoteservice/composer.json Outdated Show resolved Hide resolved
src/quoteservice/public/index.php Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Sep 8, 2022

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Sep 8, 2022
@julianocosta89 julianocosta89 marked this pull request as ready for review September 8, 2022 07:24
@julianocosta89 julianocosta89 requested a review from a team September 8, 2022 07:24
@julianocosta89 julianocosta89 changed the title [WIP] - Add PHP quote service Add PHP quote service Sep 8, 2022
@julianocosta89

This comment was marked as outdated.

@julianocosta89 julianocosta89 added enhancement New feature or request and removed in-progress This issue is actively being worked on labels Sep 8, 2022
Copy link
Contributor

@mic-max mic-max left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for tackling the beast that is PHP ;p
Update the issue this resolves since we decided PHP to be a quote service instead of an admin portal 👍

docker-compose.yml Outdated Show resolved Hide resolved
src/quoteservice/Dockerfile Outdated Show resolved Hide resolved
src/quoteservice/Dockerfile Outdated Show resolved Hide resolved
src/quoteservice/composer.json Outdated Show resolved Hide resolved
src/quoteservice/composer.json Outdated Show resolved Hide resolved
@cartersocha
Copy link
Contributor

@brettmc would you mind doing a final review on this?

src/quoteservice/Dockerfile Outdated Show resolved Hide resolved
src/quoteservice/Dockerfile Outdated Show resolved Hide resolved
Juliano Costa and others added 6 commits September 9, 2022 10:54
Co-authored-by: Michael Maxwell <mike.ian.maxwell@gmail.com>
Co-authored-by: Brett McBride <brett.a.mcbride@gmail.com>
Co-authored-by: Brett McBride <brett.a.mcbride@gmail.com>
@julianocosta89
Copy link
Member Author

There is no load to the service yet, but we can test in this way ATM:

docker compose up quoteservice --build
Once everything is up and running:

docker run --rm --network opentelemetry-demo curlimages/curl -d '{"numberOfItems":"4"}' -H "Content-Type: application/json" -X POST http://quoteservice:8090/getquote

You can change the numberOfItems to get different quotes.
Simple, but it does the job for now.

@julianocosta89
Copy link
Member Author

@brettmc everything should be good to go 🤩
Thank you very much for all the help!

@cartersocha
Copy link
Contributor

@julianocosta89 great work on this. Are we ready to merge or are there outstanding items? I see the docs are mostly updated and we can clean up the rest post merge

@julianocosta89
Copy link
Member Author

@cartersocha all good to go!
Whenever this gets merged I can start working on the shippingservice call to this service.

@cartersocha cartersocha merged commit 860a35d into open-telemetry:main Sep 14, 2022
@julianocosta89 julianocosta89 deleted the feature/add-php-quoteservice branch September 14, 2022 05:02
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants