Skip to content
This repository was archived by the owner on May 23, 2024. It is now read-only.

expose gunicorn logging #219

Merged
merged 1 commit into from
Mar 5, 2022
Merged

expose gunicorn logging #219

merged 1 commit into from
Mar 5, 2022

Conversation

mseth10
Copy link
Contributor

@mseth10 mseth10 commented Mar 2, 2022

Issue #, if available:
Expose option to set gunicorn logging level. Helpful for debugging gunicorn related errors.

Description of changes:
Changes tested in aws/deep-learning-containers#1713

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mseth10 mseth10 requested a review from satishpasumarthi March 2, 2022 02:12
@sagemaker-bot
Copy link

AWS CodeBuild CI Report

  • CodeBuild project: sagemaker-tensorflow-serving-container-pr
  • Commit ID: c3356ba
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

satishpasumarthi
satishpasumarthi approved these changes Mar 2, 2022
View reviewed changes
Copy link
Contributor

@satishpasumarthi satishpasumarthi left a comment

Choose a reason for hiding this comment

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

LGTM !

@satishpasumarthi
Copy link
Contributor

nit: Let's highlight this option in the README.md file so that our customers are aware of this for debugging?

@mseth10
Copy link
Contributor Author

mseth10 commented Mar 2, 2022

nit: Let's highlight this option in the README.md file so that our customers are aware of this for debugging?

Is there any particular section where we talk about different environment variables? If not, do you want me to add something like "Now you can use the environment variable SAGEMAKER_GUNICORN_LOGLEVEL to set the logging level for Gunicorn." in this section https://github.com/aws/sagemaker-tensorflow-serving-container#deploying-to-multi-model-endpoint @satishpasumarthi

@satishpasumarthi
Copy link
Contributor

Yeah. That would be better. Let's create a section for it if not present !

@mseth10 mseth10 requested a review from josephevans March 4, 2022 22:15
josephevans
josephevans approved these changes Mar 5, 2022
View reviewed changes
Copy link
Contributor

@josephevans josephevans left a comment

Choose a reason for hiding this comment

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

LGTM!

@mseth10 mseth10 merged commit 387ae61 into aws:master Mar 5, 2022
@mseth10 mseth10 deleted the expose-gunicorn-logging branch March 5, 2022 01:43
matherit pushed a commit to matherit/sagemaker-tensorflow-serving-container that referenced this pull request Mar 7, 2022
@mseth10 @matherit
expose gunicorn logging (aws#219)
984455c
matherit added a commit that referenced this pull request Mar 8, 2022
@matherit @mseth10
Add gunicorn timeout (#220)
a58583d 
* Add env var that will manage GUnicorn setup timeout.

* Ensure env var is treated as innt.

* Update readme to include new env var section and update TOC.

* Add SAGEMAKER_GUNICORN_LOGLEVEL description to readme.

* expose gunicorn logging (#219)

* Add in SAGEMAKER_GUNICORN_TIMEOUT_SECONDS.

* Update readme with PR feedback.

* Remove extra argument in format.

* Remove setup timeout.

* Make readme links clickable.

* Remove boiler plate statements and instead link in descriptive comment of var.

* Update capitalization of Gunicorn.

Co-authored-by: Manu Seth <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@josephevans josephevans josephevans approved these changes

@satishpasumarthi satishpasumarthi satishpasumarthi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mseth10 @sagemaker-bot @satishpasumarthi @josephevans