-
Notifications
You must be signed in to change notification settings - Fork 79
Update HTTP Handler to primarily be useriden driven, add user logging to requests (SYN-4602) #3007
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
Conversation
…andler (SYN-4602)
…on. Add many docstrings.
Codecov ReportBase: 97.20% // Head: 97.09% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #3007 +/- ##
==========================================
- Coverage 97.20% 97.09% -0.11%
==========================================
Files 220 220
Lines 43477 43528 +51
==========================================
+ Hits 42262 42264 +2
- Misses 1215 1264 +49
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@@ -33,7 +33,14 @@ | |||
MAX_SPOOL_SIZE = CHUNK_SIZE * 32 # 512 mebibytes | |||
MAX_HTTP_UPLOAD_SIZE = 4 * s_const.tebibyte | |||
|
|||
class AxonHttpUploadV1(s_httpapi.StreamHandler): | |||
class AxonHandlerMixin: | |||
def getAxon(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we want to make this async so that a remote axon ( via telepath Client ) could be awaited for ready status?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do that; I was using the existing pattern we have for the Storm handlers which is a sync call. An implementation that references a telepath Client would implicitly have a waitready() call in it, so we may be doing double duty there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a separate story or still relevant to this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Cisphyx I think that would be seperate?
…ard, use that for logging calls. Fix storm documentation
No description provided.