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

Clarify max body size units and format readme #695

Closed
wants to merge 1 commit into from

Conversation

AbhiMisRaw
Copy link

@AbhiMisRaw AbhiMisRaw commented Jan 7, 2024

It's my first contribution, as @SebCorbin said, It's an docs issue, so I've updated it BODY -> BYTE

Fixes #300

Copy link
Member

@50-Course 50-Course left a comment

Choose a reason for hiding this comment

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

Hey @AbhiMisRaw, welcome to the django-silk! Awesome first contribution—thanks for catching that docs issue and making the update to BYTE. It looks perfect!

Copy link

codecov bot commented Jan 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (23ff43b) 86.65% compared to head (c43a471) 86.65%.
Report is 4 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #695   +/-   ##
=======================================
  Coverage   86.65%   86.65%           
=======================================
  Files          52       52           
  Lines        2113     2113           
=======================================
  Hits         1831     1831           
  Misses        282      282           

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

Comment on lines +429 to +430
SILKY_MAX_REQUEST_BYTE_SIZE = -1 # Silk takes anything <0 as no limit
SILKY_MAX_RESPONSE_BYTE_SIZE = 1024 # If response body>1024 bytes, ignore
Copy link
Member

Choose a reason for hiding this comment

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

SILKY_MAX_REQUEST_BODY_SIZE is being read within django-silk: https://github.com/search?q=repo%3Ajazzband%2Fdjango-silk%20SILKY_MAX_RESPONSE_BODY_SIZE&type=code

The variable cannot be changed to SILKY_MAX_RESPONSE_BYTE_SIZE in the readme without updating the code within django-silk (which would be a bad idea because it would require users of django-silk to update their code as it would be backwards incompatible)

@albertyw albertyw changed the title fix issue #300 Clarify max body size units and format readme (#300) Jan 15, 2024
@albertyw albertyw changed the title Clarify max body size units and format readme (#300) Clarify max body size units and format readme Jan 15, 2024
@albertyw albertyw closed this Jun 10, 2024
@albertyw
Copy link
Member

The issue with BODY/BYTES was fixed in d40bb9d. The formatting changes are fine but are separate from what this PR was trying to fix.

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.

Max body size units
3 participants