-
Notifications
You must be signed in to change notification settings - Fork 428
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
logger rework. Package local logger. #1583
Conversation
def emit(self, record): | ||
pass | ||
|
||
logging.getLogger(__name__).addHandler(NullHandler()) |
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.
Maybe you're not done with this PR yet, so this could be a premature comment.
Using NullHandler is exactly the right thing to do for a library. It makes your library a good logging citizen. Also, the package's root-level __init__.py
file is almost always the right place to invoke the basic logging configuration. (In conda's case I'm not, because I don't want to bog down the ..activate
etc cli stuff with logger initialization. It's way too slow already.)
However, unless you add another handler somewhere else, the problem with conda-build logs will remain. I'm guessing you're just not done with the PR though.
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.
Thanks for the comment. I'm not sure why, but things seem to be working with the code as is. Any insight?
I have a package on my channel, intentionally built with an 80-character prefix. This recipe will try to bring it in as a dependency:
package:
name: test-pkg
version: 1.0
requirements:
build:
- conda-build-test-has-prefix-files
Now, if you build that recipe (conda build -c msarahan .
), you see the logger output, as I would hope. is there something else that isn't showing because of the way I have things now?
msarahan@0109-msarahan-2 ~/code/conda-build/tests/test_pkg logger conda build -c msarahan .
BUILD START: test-pkg-1.0-0
updating index in: /Users/msarahan/miniconda2/conda-bld/osx-64
updating index in: /Users/msarahan/miniconda2/conda-bld/noarch
The following packages will be downloaded:
package | build
---------------------------|-----------------
conda-build-test-has-prefix-files-1.0| 0 3 KB local
The following NEW packages will be INSTALLED:
conda-build-test-has-prefix-files: 1.0-0 local
WARNING conda_build.build:create_env(648): Build prefix failed with prefix length 255
WARNING conda_build.build:create_env(649): Error was:
WARNING conda_build.build:create_env(650): Placeholder of length '80' too short in package local::conda-build-test-has-prefix-files-1.0-0.
The package must be rebuilt with conda-build > 2.0.
WARNING conda_build.build:create_env(651): One or more of your package dependencies needs to be rebuilt with a longer prefix length.
WARNING conda_build.build:create_env(653): Falling back to legacy prefix length of 80 characters.
WARNING conda_build.build:create_env(654): Your package will not install into prefixes > 80 characters.
Hi there, thank you for your contribution! This pull request has been automatically locked because it has not had recent activity after being closed. Please open a new issue or pull request if needed. Thanks! |
Fixes logger messages not showing up due to root logger being configured at higher logging level. Localizes logger configuration to conda-build.
This probably trims things back too far. Running conda-build in debug mode should probably also run conda in debug mode, for example. I leave that for a future PR.
CC @mingwandroid @kalefranz