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

Remove files list from status info; update README.md #22

Merged
merged 3 commits into from
Oct 20, 2023

Conversation

webbnh
Copy link
Member

@webbnh webbnh commented Oct 20, 2023

Now that we're getting ready to deploy this in production, having the current list of files available is a security hazard. This PR removes that (leaving the disk utilization in the response, because that seems innocuous and even generally useful), and it brings the README.md file up to date accordingly.

@webbnh webbnh requested a review from dbutenhof October 20, 2023 19:33
@webbnh webbnh self-assigned this Oct 20, 2023
@webbnh webbnh force-pushed the update-status-info branch from de3918d to 79b8c13 Compare October 20, 2023 19:37
@webbnh webbnh marked this pull request as draft October 20, 2023 19:38
@webbnh
Copy link
Member Author

webbnh commented Oct 20, 2023

Apparently I actually had tests for this stuff...converting to draft until I have them straightened out. 🥴

dbutenhof
dbutenhof previously approved these changes Oct 20, 2023
Copy link
Member

@dbutenhof dbutenhof left a comment

Choose a reason for hiding this comment

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

S'OK, but a suggestion to make it more of an API ...

@codecov
Copy link

codecov bot commented Oct 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0d3d1db) 98.52% compared to head (18dd8cb) 98.47%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #22      +/-   ##
==========================================
- Coverage   98.52%   98.47%   -0.05%     
==========================================
  Files           2        2              
  Lines         541      526      -15     
==========================================
- Hits          533      518      -15     
  Misses          8        8              
Files Coverage Δ
src/relay/relay.py 100.00% <100.00%> (ø)
tests/test_relay.py 97.90% <100.00%> (-0.06%) ⬇️

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

dbutenhof
dbutenhof previously approved these changes Oct 20, 2023
Copy link
Member

@dbutenhof dbutenhof left a comment

Choose a reason for hiding this comment

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

If you're improving the response, that's great; but I'm tossing a +1 anyway.

Copy link
Member

@dbutenhof dbutenhof left a comment

Choose a reason for hiding this comment

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

Yeah, though, by the way, this really should have had a Jira issue. 👀

@webbnh webbnh marked this pull request as ready for review October 20, 2023 20:55
@webbnh webbnh merged commit c98008e into distributed-system-analysis:main Oct 20, 2023
@webbnh webbnh deleted the update-status-info branch October 20, 2023 21:02
@webbnh
Copy link
Member Author

webbnh commented Oct 20, 2023

this really should have had a Jira issue

I attached it to PBENCH-1258.

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.

2 participants