Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixing server registration #647

Merged
merged 3 commits into from
Jun 1, 2021
Merged

Fixing server registration #647

merged 3 commits into from
Jun 1, 2021

Conversation

shahafn
Copy link
Member

@shahafn shahafn commented May 30, 2021

Now server validates stake status with hub before trying to add
workers/register

shahafn added 2 commits May 20, 2021 05:55
Now server validates stake status with hub before trying to add
workers/register
@@ -450,7 +450,6 @@ export class ContractInteractor {
}
const rangeSize = toBlock - fromBlock + 1
const pagesForRange = Math.max(Math.ceil(rangeSize / this.maxPageSize), 1)
this.logger.info(`Splitting request for ${rangeSize} blocks into ${pagesForRange} smaller paginated requests!`)
Copy link
Member

Choose a reason for hiding this comment

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

  1. No need to log if doing nothing (single part). If you think logging is necessary, you can log if me parts are there.
  2. Logging should use debug. Info is for one-time (e.g startup code) or highly important (e.g actual sending of a new user tx)
    (That is, if you think this log is ever required. Otherwise, it indeed can be removed)

@drortirosh drortirosh merged commit d2479ec into master Jun 1, 2021
@drortirosh drortirosh deleted the server-registration-fix branch June 1, 2021 22:27
@forshtat
Copy link
Member

forshtat commented Jun 6, 2021

@shahafn @drortirosh I see that this PR removes the 'Splitting request for ${rangeSize} blocks' logs entirely, doesn't it? Don't you think these logs might be useful for us later? Maybe it should remain but only print if rangesCount > 1 ?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants