Adds log level option and configuration via environment in scripts.#271
Adds log level option and configuration via environment in scripts.#271cleitonme wants to merge 1 commit intoasternic:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the application's logging capabilities by introducing flexible log level configuration. Users can now specify the desired logging verbosity through a new command-line flag or an environment variable, allowing for easier debugging and operational control without modifying code. This change improves observability and adaptability of the application in various environments. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a configurable log level, which is a great enhancement. The implementation is mostly solid, with a new command-line flag and environment variable to control the log level. I've made a few suggestions to improve consistency and configurability:
- In
main.go, the precedence of the environment variable over the command-line flag is inconsistent with other settings in the application. I've suggested a change to align this. - In
docker-compose.ymlanddocker-compose-swarm.yaml, theLOG_LEVELis hardcoded. I've suggested using variable substitution to allow it to be configured from the environment, which is the pattern used for other variables.
| - TZ=${TZ:-America/Sao_Paulo} | ||
| - WEBHOOK_FORMAT=${WEBHOOK_FORMAT:-json} | ||
| - SESSION_DEVICE_NAME=${SESSION_DEVICE_NAME:-WuzAPI} | ||
| - LOG_LEVEL=info |
There was a problem hiding this comment.
The LOG_LEVEL is hardcoded. It should use variable substitution to allow configuration from the shell environment, which is the standard practice for swarm deployments as noted in the README. This provides flexibility for different environments.
- LOG_LEVEL=${LOG_LEVEL:-info}| - TZ=${TZ:-America/Sao_Paulo} | ||
| - WEBHOOK_FORMAT=${WEBHOOK_FORMAT:-json} | ||
| - SESSION_DEVICE_NAME=${SESSION_DEVICE_NAME:-WuzAPI} | ||
| - LOG_LEVEL=info |
There was a problem hiding this comment.
The LOG_LEVEL is hardcoded to info. To allow for easier configuration via the .env file, it should use variable substitution with a default value, similar to other environment variables in this file. This improves configurability.
- LOG_LEVEL=${LOG_LEVEL:-info}| if v := os.Getenv("LOG_LEVEL"); v != "" { | ||
| *logLevel = v | ||
| } |
There was a problem hiding this comment.
The current implementation for setting the log level from an environment variable gives it higher precedence than the command-line flag. This is inconsistent with how other configurations like WUZAPI_ADDRESS and WUZAPI_PORT are handled in this file, where command-line flags override environment variables. To maintain consistency, the logic should be adjusted to only use the environment variable if the command-line flag has not been set (i.e., it has its default value of "info"). You might also want to update the comment on line 184 to reflect this change, for example: // Configure log level from environment variable if flag is default.
| if v := os.Getenv("LOG_LEVEL"); v != "" { | |
| *logLevel = v | |
| } | |
| if v := os.Getenv("LOG_LEVEL"); v != "" && *logLevel == "info" { | |
| *logLevel = v | |
| } |
1 participant
No description provided.