feat: add go/batch-submitter env var parsing#1537
feat: add go/batch-submitter env var parsing#1537tynes merged 2 commits intoethereum-optimism:developfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1537 +/- ##
========================================
Coverage 76.71% 76.71%
========================================
Files 82 82
Lines 3032 3032
Branches 463 463
========================================
Hits 2326 2326
Misses 706 706
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
|
Hey @optimisticben - any opinions on this new config scheme? Its very similar to the existing one |
tynes
left a comment
There was a problem hiding this comment.
Will be good to merge with the removal of DisableQueueBatchAppend
72df96c to
bd96432
Compare
|
Opinions? Always. We need this? I'm used to seeing a chain id
Flag names nits
ENV names for shared configs (maybe outside scope)I would really like to see configurations that should be common across the whole system defined in a way that they can be easily shared. |
If you have strong preference for removing the prefixes for the env vars then I'm fine with it. It was something that we planned on doing to prevent accidental configurations. Sorry for telling you to do that conner, it should be easy to remove that add prefix function in the flags |
bd96432 to
fde1de3
Compare
Currently this is only used for Sentry logs and Prometheus labels, is chain id still preferable there?
Done!
I can definitely add this as well. Tried to comb through and ended up with this list:
I removed those prefixes, lmk what you think.
No problem, easy change! Updated ./batch-submitter -h output |
fde1de3 to
f18e676
Compare
|
LGTM, thanks! |
|
I've noticed that you added in a changesets Line 12 in e5c2686 This will allow us to run |
f18e676 to
ad91500
Compare
|
It looks like there is a problem due to name collisions. Perhaps we rename the new one to I believe it needs to be updated here: |
tynes
left a comment
There was a problem hiding this comment.
Once the changeset name is updated, this should be good to go
ad91500 to
f96f611
Compare
|
@tynes updated to |
|
Great work! |
Description
This PR adds parsing for the environment variables to be used in the upcoming rewrite of the batch submitter in Go. The currently selected set is mostly the same as the existing ones, though with some historical or unneeded parameters removed. In addition, this PR sets up unit testing and linting for the
go/batch-submitterdirectory in Github Actions.Additional context
Example output of
./batch-submitter -hMetadata