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

enhance info endpoint and bump to v0.1.13 #117

Merged
merged 9 commits into from
Mar 5, 2024

Conversation

LiranCohen
Copy link
Member

@LiranCohen LiranCohen commented Mar 4, 2024

In a previous PR some(much desired) version information was added to the /info endpoint, however in certain environments(our docker container) this blows up.

This PR prevents the endpoint from blowing up, while also providing some optionality and defaults to fall back on and bumps version to v0.1.13

Added the text below to the README.md file:

  • server is read from the process.env.npm_package_name variable that npm provides. If that does not exist, it will check for a DWN_SERVER_PACKAGE_NAME environment variable set by the user, or otherwise it will default to @web5/dwn-server.
  • version and sdkVersion are read from the package.json file. It will locate the file's path either from the process.env.npm_package_json variable that npm provides. If that does not exist, it will check for a DWN_SERVER_PACKAGE_JSON environment variable set by the user, or otherwise it will default to /dwn-server/package.json which is the path within the default Docker container build.

@codecov-commenter
Copy link

codecov-commenter commented Mar 4, 2024

Codecov Report

Attention: Patch coverage is 70.83333% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 94.23%. Comparing base (40e758c) to head (d78eba3).

Files Patch % Lines
src/http-api.ts 69.56% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #117      +/-   ##
==========================================
- Coverage   94.45%   94.23%   -0.23%     
==========================================
  Files          25       25              
  Lines        2218     2237      +19     
  Branches      296      299       +3     
==========================================
+ Hits         2095     2108      +13     
- Misses        123      129       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

thehenrytsai
thehenrytsai previously approved these changes Mar 5, 2024
Copy link
Collaborator

@thehenrytsai thehenrytsai left a comment

Choose a reason for hiding this comment

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

At least it is an improvement to blowing up. Though doesn't this depend on DWN_SERVER_VERSION being available? I wonder if it is not possible to just read the package.json file direclty.

@LiranCohen LiranCohen changed the title enhance info endpoint enhance info endpoint and bump to v0.1.13 Mar 5, 2024
@LiranCohen
Copy link
Member Author

@thehenrytsai spent a little more time on it to make this a more thoughtful PR. Added some notes to the readme.

@LiranCohen LiranCohen requested a review from thehenrytsai March 5, 2024 16:28
src/http-api.ts Outdated Show resolved Hide resolved
src/config.ts Outdated Show resolved Hide resolved
@LiranCohen LiranCohen requested a review from thehenrytsai March 5, 2024 19:52
Copy link
Collaborator

@thehenrytsai thehenrytsai left a comment

Choose a reason for hiding this comment

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

🎸 🎸 🎸

@LiranCohen LiranCohen merged commit 3436b21 into main Mar 5, 2024
8 checks passed
@LiranCohen LiranCohen deleted the lirancohen/fix-info-endpoint branch March 5, 2024 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants