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

Remove redundant usage of config load across codebase and refactor logging #44

Open
lucifercr07 opened this issue Oct 21, 2024 · 8 comments · May be fixed by #53
Open

Remove redundant usage of config load across codebase and refactor logging #44

lucifercr07 opened this issue Oct 21, 2024 · 8 comments · May be fixed by #53
Assignees

Comments

@lucifercr07
Copy link
Contributor

@srivastava-yash
Copy link

I want to give it a go, can you assign it to me?

@lucifercr07
Copy link
Contributor Author

@srivastava-yash assigned, thanks for contributing. Feel free to refactor if you find any other issues.

@srivastava-yash
Copy link

@lucifercr07 Do you think if we want to keep log library similar to that then we can have the log library as the package itself that can be imported at both places?

@lucifercr07
Copy link
Contributor Author

@srivastava-yash it doesn't need to be exact same as dicedb, we can tune here as per service requirement. Just added that for reference.

@lucifercr07
Copy link
Contributor Author

@srivastava-yash any updates on this?

@srivastava-yash
Copy link

@lucifercr07 i have completed 1 of the 2 tasks. Standardized the config. Working on the logging part now. Also do we need to integration of zerolog with slog here? Or just slog is enough? I believe zerolog was used in dicedb for its performance, do we have the same requirement here?

@lucifercr07
Copy link
Contributor Author

@lucifercr07 i have completed 1 of the 2 tasks. Standardized the config. Working on the logging part now. Also do we need to integration of zerolog with slog here? Or just slog is enough? I believe zerolog was used in dicedb for its performance, do we have the same requirement here?

slog should be enough for now, also check the latest usage in DiceDB, we don't need to pass log instances everywhere.

@srivastava-yash srivastava-yash linked a pull request Oct 30, 2024 that will close this issue
2 tasks
@srivastava-yash
Copy link

@lucifercr07 , I have raised a PR for this, let me know your comments on it when you are free. Thanks.

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 a pull request may close this issue.

2 participants