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

ujson instead of json #66

Closed
wants to merge 1 commit into from
Closed

Conversation

sjamgade
Copy link
Contributor

No description provided.

@Ormod
Copy link
Contributor

Ormod commented Mar 16, 2021

I'm not too keen on the idea, the JSON files that pglookout touches are minimal i.e. I doubt you'll be able to measure any performance impact from this. This in comparison to adding a new dependency that needs to be kept up to date. Also your commit doesn't have any rationale for why you want to actually do this.

@sjamgade
Copy link
Contributor Author

Agreed the advantage is on lower side and dependency addition further lowers the overall benefit. If the other PR (draining the queue) is not the correct way ahead then when the queue has filled with pending items, this will help speed up the consumer thread

@sjamgade
Copy link
Contributor Author

#65 got merged, so this is not needed. Closing.

@sjamgade sjamgade closed this Mar 22, 2021
@sjamgade sjamgade deleted the use-ujson branch March 22, 2021 08:18
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