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.

Sign up for GitHub

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

Jump to bottom

Implement the show endpoint in the httpd crate #175

Merged
merged 4 commits into from
Jul 24, 2023
Merged

Implement the show endpoint in the httpd crate #175

merged 4 commits into from
Jul 24, 2023

Conversation

tareknaser
Copy link
Collaborator

  • fixes the links to crates in the README file
  • define a new type Show to send to the show http endpoint
  • implement coffee_show function to handle /show endpoint
  • add an httpd integration test to further stress test the server.

See commit body for details

@netlify
Copy link

netlify bot commented Jul 19, 2023 •
edited
Loading

✅ Deploy Preview for coffee-docs canceled.

Name Link
🔨 Latest commit f066f27
🔍 Latest deploy log https://app.netlify.com/sites/coffee-docs/deploys/64bded50a15ca9000885f1be

vincenzopalazzo
vincenzopalazzo requested changes Jul 19, 2023
View reviewed changes
Copy link
Contributor

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

Ok I have added the review.

This is good but lets focus on the real refactoring that we need to do

We are not stressing in this way coffee at all

coffee_httpd/src/httpd/server.rs Outdated Show resolved Hide resolved
tests/src/coffee_httpd_integration_tests.rs Outdated Show resolved Hide resolved
tests/src/coffee_httpd_integration_tests.rs Outdated Show resolved Hide resolved
tests/src/coffee_httpd_integration_tests.rs Outdated Show resolved Hide resolved
vincenzopalazzo
vincenzopalazzo requested changes Jul 20, 2023
View reviewed changes
tests/src/coffee_httpd_integration_tests.rs Outdated Show resolved Hide resolved
tests/src/coffee_httpd_integration_tests.rs Outdated Show resolved Hide resolved
vincenzopalazzo
vincenzopalazzo reviewed Jul 21, 2023
View reviewed changes
Copy link
Contributor

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

LGTM, this needs only to fix the commit history in three

  1. modify the macro
  2. implement the endpoint
  3. add testing for the endpoint

@tareknaser
fix(docs): fix links to crates in README
93e0296 
This commit:
1- fixes the links to crates in the readme file
2- fixes some typos within the readme file

Signed-off-by: Tarek <[email protected]>
@tareknaser
fix(httpd): modify the macro to use in all endpoints
2e17841 
Signed-off-by: Tarek <[email protected]>
@tareknaser
feat(httpd): implement the show endpoint
efe02dc 
This commit:
1- define a new type Show to send to the show http endpoint
2- implement coffee_show function to handle /show endpoint
3- modify the /list and /remote/list endpoints to use the macro

Signed-off-by: Tarek <[email protected]>
@tareknaser
feat(tests): add a new httpd test
f066f27 
This commit:
1- adds an httpd integration test to further stress test the server. The new test mimics a test case where all endpoints are called.
2- implements Debug for CoffeeHTTPDTesting

Signed-off-by: Tarek <[email protected]>
vincenzopalazzo
vincenzopalazzo approved these changes Jul 24, 2023
View reviewed changes
Copy link
Contributor

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

LGTM

@vincenzopalazzo vincenzopalazzo merged commit 5224e63 into coffee-tools:master Jul 24, 2023
9 checks passed
@tareknaser tareknaser mentioned this pull request Jul 25, 2023
Closed
@tareknaser tareknaser deleted the show_httpd branch July 31, 2023 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vincenzopalazzo vincenzopalazzo vincenzopalazzo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tareknaser @vincenzopalazzo