-
Notifications
You must be signed in to change notification settings - Fork 863
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
Security documentation update #3183
Conversation
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 should make this more prominent so users are aware of this. Please also add a prominent logging message about the changes when torchserve starts (framed in ##### or something). Also add where the keyfile is located ( and if not already there add an example on how to send requests to models now)
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.
Minor comment but otherwise this LGTM now
frontend/server/src/main/java/org/pytorch/serve/http/TokenAuthorizationHandler.java
Show resolved
Hide resolved
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.
Please align the argument name to --enable-model-api and make sure you're usage is right in the doc. AFAIK --enable_model_api won't work.
frontend/server/src/main/java/org/pytorch/serve/util/ConfigManager.java
Outdated
Show resolved
Hide resolved
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.
See comment
frontend/server/src/main/java/org/pytorch/serve/util/ConfigManager.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Naman Nandan <namankt55@gmail.com>
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.
LGTM
Description
Documentation update for security PRs #3163 and #3165
To be merged after both security PRs are merged