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

Setting Config.ProgressReportFrequency to a small or negative number would be bad #214

Open
milanatshopify opened this issue Oct 5, 2020 · 2 comments

Comments

@milanatshopify
Copy link
Contributor

There is nothing in the code that protects us from misconfiguration of ProgressReportFrequency. Setting it to zero or a negative number, for example. There should probably be a minimum allowable value, where we we either clamp to that minimum, or we skip the progress reporting completely if the value is lower.

@xliang6

@milanatshopify milanatshopify changed the title Setting Config.ProgressReportFrequency to a 0 or negative number would be bad Setting Config.ProgressReportFrequency to a small or negative number would be bad Oct 5, 2020
@shuhaowu
Copy link
Contributor

shuhaowu commented Oct 5, 2020

So a simple fix like this one? #215 😛

@milanatshopify
Copy link
Contributor Author

So a simple fix like this one? #215 😛

It could be as simple as that! In particular, because the value is ms, it may be easy for somebody to set a value off by 100, and end up with something way too frequent. A reasonable minimum would take care of that. I won't comment on whether 1 minute is a reasonable minimum :)

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

No branches or pull requests

2 participants