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

Add documentation for the omero.logging properties #6393

Merged
merged 3 commits into from
Jul 12, 2024

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Jun 19, 2024

Initial step towards fixing #6225

All these properties are defined in OMERO.py so changing the default value in this file has no effect but allows for the configuration to be included in the reference documentation

All these properties are defined in OMERO.py so changing the default
value in this file has no effect but allows for the configuration
to be included in the reference documentation
@will-moore
Copy link
Member

Looks good. Text makes sense.

So this PR is purely to update the docs at https://omero.readthedocs.io/en/latest/sysadmins/config.html?
Trivial point: I noticed there that all booleans tend to be lowercase false whereas you have a mix (although I think it doesn't matter since they are all lowercased when comparing, but just for consistency).

@sbesson sbesson force-pushed the logging_properties branch from d3f24f5 to b73a17c Compare June 21, 2024 10:11
@sbesson
Copy link
Member Author

sbesson commented Jun 21, 2024

So this PR is purely to update the docs at https://omero.readthedocs.io/en/latest/sysadmins/config.html?

Absolutely, the default logging values are defined in https://github.com/ome/omero-py/blob/8d8108de02104c654741d65f0ecbbf11c85e329d/src/omero/util/__init__.py#L32-L38 so modifying etc/omero.properties has no functional effect.

But it is currently undocumented that the server logs can be configured using the standard omero config approach and this PR aims to update the documentation to include these properties.

Trivial point: I noticed there that all booleans tend to be lowercase false whereas you have a mix (although I think it doesn't matter since they are all lowercased when comparing, but just for consistency).

Pushed b73a17c to use the current lowercase convention consistently

Copy link
Member

@will-moore will-moore left a comment

Choose a reason for hiding this comment

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

Thanks

@sbesson
Copy link
Member Author

sbesson commented Jun 26, 2024

While using these properties as part of the review of ome/omero-py#417, one important aspect worth noting is that the omero.logging.level property only affects the Python servers (like Processor or Tables). This is consistent with what is described in https://omero.readthedocs.io/en/stable/developers/logging.html.

The last commit should update the property description to be explicit about this.

@jburel jburel merged commit b8b9413 into ome:develop Jul 12, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Document Python logging properties
4 participants