-
Notifications
You must be signed in to change notification settings - Fork 56
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
make it easier to see ES docs when debugging wrapper #81
make it easier to see ES docs when debugging wrapper #81
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.
Traditional way to do this would be by adding arguments such as v
or vv
using argparser and then setting loglevel as commented. It's just a suggestion to improve things, but this is a nice add @bengland2
run_snafu.py
Outdated
@@ -35,7 +35,12 @@ | |||
urllib3_log = logging.getLogger("urllib3") | |||
urllib3_log.setLevel(logging.CRITICAL) | |||
|
|||
setup_loggers("snafu", logging.DEBUG) | |||
if os.getenv("snafu_debug") != None: |
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.
Instead of this, can we follow the methodology that most python programs follow i.e. through simple argparse:
parser.add_argument(
"-v",
"--verbose",
dest="loglevel",
help="set loglevel to INFO",
action="store_const",
const=logging.INFO)
parser.add_argument(
"-vv",
"--very-verbose",
dest="loglevel",
help="set loglevel to DEBUG",
action="store_const",
const=logging.DEBUG)
run_snafu.py
Outdated
setup_loggers("snafu", logging.DEBUG) | ||
logger.info("logging level is DEBUG") | ||
else: | ||
setup_loggers("snafu", logging.INFO) |
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.
and then this can be
setup_loggers("snafu", args.loglevel)
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.
+1
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.
that's ok with me, my point is that there should be SOME way other than code change to have run_snafu.py turn on display of documents that it's pushing to ES. And I'll add change to documentation to explain how to do this.
set log level to DEBUG if --verbose parameter passed, otherwise default it to INFO
913308e
to
f002202
Compare
The above commit implements aakarsh's suggestion. OK? |
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.
This is neat, nice work @bengland2
set log level to DEBUG if snafu_debug env. var. defined
otherwise set it to INFO