Skip to content
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

Merged
merged 1 commit into from
Oct 21, 2019

Conversation

bengland2
Copy link
Contributor

set log level to DEBUG if snafu_debug env. var. defined
otherwise set it to INFO

Copy link
Contributor

@aakarshg aakarshg left a 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:
Copy link
Contributor

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)
Copy link
Contributor

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) 

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

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
@bengland2
Copy link
Contributor Author

The above commit implements aakarsh's suggestion. OK?

Copy link
Contributor

@aakarshg aakarshg left a 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

@aakarshg aakarshg merged commit 6cfe78a into cloud-bulldozer:master Oct 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants