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

Logging custom docs #515

Closed
wants to merge 7 commits into from
Closed

Conversation

euri10
Copy link
Member

@euri10 euri10 commented Dec 9, 2019

attempt at solving #491

It adds a simple example of an app with custom logging

I added markdown-include so that examples are code, it could be a good idea to add to the tests CI that part of the docs

@tomchristie
Copy link
Member

Great work getting started on this!

It'd be great if we could make this a bit more minimal. In particular:

  • It'd be helpful to just give an example logging config file that replicates the existing default log config.
  • It'd be nice if the only things that changed in the PR were (1) Adding an example log config file. and (2) linking to it from the settings docs.

I'd prefer it if we could drop all the other surrounding context.

@euri10
Copy link
Member Author

euri10 commented Dec 9, 2019

ok I made the logging_example.yml replicate the default config the only difference being it's a yaml file and not a dict
I kept markdown-include addition but I can remove it, it just makes things simpler imho in the case we put a yaml file as a config, any change in it will be automatically reflected in the docs, but I understand if you'd prefer not to.

Next slash in the PR could be to just copy/paste the LOGGING_CONFIG dictionnary into a file and link it in the settings page, that;d more minimal, let me know what you prefer :)

docs/examples.md Outdated

If you used the `--use-colors / --no-use-colors` then the output will / won't be colorized.

```yaml hl_lines="7 11 38 39 40 41"
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather than any example is just included directly, than using this style.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok so you prefer to remove markdown-include and just copy-paste examples right ?

Copy link
Member

Choose a reason for hiding this comment

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

Yup

Copy link
Member Author

Choose a reason for hiding this comment

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

done

docs/examples.md Outdated
## Custom logging

The below `logging_example.yaml` is a yaml representation of the default logging configuration.
You can pass it loading a dict, using the `--log-config` flag.
Copy link
Member

Choose a reason for hiding this comment

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

Just checking... Does this definately work?
I haven't checked out if Python's log file format also supports using the yaml style?

Copy link
Member Author

Choose a reason for hiding this comment

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

this is definitely an ambiguous formulation, python logging supports dict / file (ini style). A lot of time people use yaml and load it to dict with pyyaml for instance, that's why I said "representation"

Copy link
Member

Choose a reason for hiding this comment

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

Right, but it wouldn't directly work with our --log-config flag right?

(Tho we could potentially change things here, so that we expect a YAML style logging config file.)

Copy link
Member Author

Choose a reason for hiding this comment

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

correct, one would have to do something like

    with open('logging_example.yaml', 'r') as stream:
        config = yaml.load(stream, yaml.FullLoader)
    uvicorn.run("main:app", reload=True, log_config=config, log_level="trace")

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.

2 participants