Skip to content

Add DIFF_BLOCK_FROM_HEAD_OF_CHAIN to execute docker startup script#184

Merged
elizabethengelman merged 4 commits intostagingfrom
vdb-1312-limit-diff-processing
Apr 30, 2020
Merged

Add DIFF_BLOCK_FROM_HEAD_OF_CHAIN to execute docker startup script#184
elizabethengelman merged 4 commits intostagingfrom
vdb-1312-limit-diff-processing

Conversation

@elizabethengelman
Copy link
Copy Markdown
Contributor

@elizabethengelman elizabethengelman commented Apr 30, 2020

@elizabethengelman elizabethengelman force-pushed the vdb-1312-limit-diff-processing branch from 7539053 to ed510a3 Compare April 30, 2020 14:49
exit 1
fi

DIFF_STARTING_BLOCK="-1"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure if this is the best approach for handling this, so would be totally up for changing it up if folks have ideas!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This approach makes sense to me 👍

Seems like someone running the binary directly could just omit that flag, but if we're always including the flag then I like having a default. My only thought would be wondering if we could use an env var instead of a flag (so that it was fully optional and didn't need to be in the dockerfiles at all), but I think that makes it harder to set a default - and potentially means we'd need to synchronize deploying this with a modification to our configured env vars

@elizabethengelman elizabethengelman force-pushed the vdb-1312-limit-diff-processing branch 4 times, most recently from a2893b0 to 7955289 Compare April 30, 2020 19:48
go.mod Outdated

replace github.com/ethereum/go-ethereum => github.com/vulcanize/go-ethereum v0.0.0-20190731183759-8e20673bd101

replace github.com/makerdao/vulcanizedb => github.com/makerdao/vulcanizedb v0.0.14-rc.1.0.20200430184332-ecfa2869a661
Copy link
Copy Markdown
Contributor Author

@elizabethengelman elizabethengelman Apr 30, 2020

Choose a reason for hiding this comment

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

Once sky-ecosystem/vulcanizedb#84 is merged, I was thinking of cutting a new VDB version and using that on line 11 instead of replacing the version.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not opposed to cutting a release and updating that way, but I think we could also solve this matter in the short term by running go get github.com/makerdao/vulcanizedb@modify-unchecked-diffs-query. Maybe nice to just modify the import directly instead of using a replace?

Copy link
Copy Markdown
Contributor

@rmulhol rmulhol left a comment

Choose a reason for hiding this comment

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

LGTM!

Would be a fan of updating the dep via go get rather than a replace unless that runs into problems

exit 1
fi

DIFF_STARTING_BLOCK="-1"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This approach makes sense to me 👍

Seems like someone running the binary directly could just omit that flag, but if we're always including the flag then I like having a default. My only thought would be wondering if we could use an env var instead of a flag (so that it was fully optional and didn't need to be in the dockerfiles at all), but I think that makes it harder to set a default - and potentially means we'd need to synchronize deploying this with a modification to our configured env vars

go.mod Outdated

replace github.com/ethereum/go-ethereum => github.com/vulcanize/go-ethereum v0.0.0-20190731183759-8e20673bd101

replace github.com/makerdao/vulcanizedb => github.com/makerdao/vulcanizedb v0.0.14-rc.1.0.20200430184332-ecfa2869a661
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not opposed to cutting a release and updating that way, but I think we could also solve this matter in the short term by running go get github.com/makerdao/vulcanizedb@modify-unchecked-diffs-query. Maybe nice to just modify the import directly instead of using a replace?

eventLogs, getLogsErr := eventLogRepository.GetUntransformedEventLogs()

logCount := getLogCount(db)
eventLogs, getLogsErr := eventLogRepository.GetUntransformedEventLogs(0, logCount)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nice catch!

@elizabethengelman elizabethengelman force-pushed the vdb-1312-limit-diff-processing branch from 7955289 to d47308b Compare April 30, 2020 21:15
@elizabethengelman elizabethengelman force-pushed the vdb-1312-limit-diff-processing branch from 407a18f to 3adc753 Compare April 30, 2020 22:09
@elizabethengelman elizabethengelman force-pushed the vdb-1312-limit-diff-processing branch from 3adc753 to f572896 Compare April 30, 2020 22:25
@elizabethengelman elizabethengelman merged commit 593d3ec into staging Apr 30, 2020
@elizabethengelman elizabethengelman deleted the vdb-1312-limit-diff-processing branch April 30, 2020 22:59
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