batch-submitter: updated config #847

tynes · 2021-05-12T05:37:20Z

Description
Updates the config in a backwards compatible way so that it works with bcfg. Now the prefix BATCH_SUBMITTER_<config-name> will be parsed as config as well as CLI arguments. This was done in a backwards compatible way such that the previous env vars are parsed as the default values. We can move towards phasing out the old configuration style in the future.

Also uses the USE_SENTRY config option to turn on sentry

changeset-bot · 2021-05-12T05:37:22Z

🦋 Changeset detected

Latest commit: 1a45645

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@eth-optimism/core-utils	Patch
@eth-optimism/data-transport-layer	Patch
@eth-optimism/message-relayer	Patch
@eth-optimism/batch-submitter	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codecov-commenter · 2021-05-12T05:57:04Z

Codecov Report

Merging #847 (1a45645) into develop (20242af) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #847   +/-   ##
========================================
  Coverage    82.21%   82.21%           
========================================
  Files           48       48           
  Lines         1895     1895           
  Branches       303      303           
========================================
  Hits          1558     1558           
  Misses         337      337

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20242af...1a45645. Read the comment docs.

annieke

this change overall looks good to me, can we update .env.example to the new format?

annieke · 2021-05-12T19:15:08Z

packages/batch-submitter/src/exec/run-batch-submitter.ts

+  const name = 'oe:batch_submitter:init'
+  let logger
+
+  if (config.bool('use-sentry')) {


to confirm, this is the only new variable added / changed in this PR?

Yes this is the only new variable added - it is used to be explicit and we should make sure that we use the same name across the different services

@tynes this defaults to false then i'm assuming? would appreciate adding this to .env.example!

packages/batch-submitter/src/exec/run-batch-submitter.ts

tynes · 2021-05-12T20:03:35Z

this change overall looks good to me, can we update .env.example to the new format?

This is backwards compatible, with updating .env.example to the new format I'd like to also update the ops directory as well as our k8s deployments and then remove the need to be backwards compatible

tynes · 2021-05-12T22:24:42Z

this change overall looks good to me, can we update .env.example to the new format?

This is backwards compatible, with updating .env.example to the new format I'd like to also update the ops directory as well as our k8s deployments and then remove the need to be backwards compatible

To follow up @annieke, I do not plan on updating the example env until the backwards compatibility is removed and we fully use the scheme of prefixed env vars. Right now the example env is acting as a forcing function for backwards compatibility

annieke

lgtm with one nit

annieke · 2021-05-12T22:33:12Z

packages/batch-submitter/src/exec/run-batch-submitter.ts

+  const name = 'oe:batch_submitter:init'
+  let logger
+
+  if (config.bool('use-sentry')) {


@tynes this defaults to false then i'm assuming? would appreciate adding this to .env.example!

tynes · 2021-05-12T23:10:14Z

I've also added a changeset for the other impacted packages

packages/batch-submitter/src/exec/run-batch-submitter.ts

* batch-submitter: backwards compatible configuration * chore: add changeset * deps: update * js: move bcfg interface to core-utils * batch-submitter: parse USE_SENTRY and add to env example * chore: add changeset * batch-submitter: parse as float instead of int * batch-submitter: better error logging

…552) * l2geth: add Backend enums and config parsing * l2geth: move OVMContext to types file * l2geth: implement syncservice spec * l2geth: fix error handling for get tx batch * l2geth: update tests to compile and pass * l2geth: add sync range functions * l2geth: add batch index indexing * l2geth: update syncservice hot path logging * l2geth: use indexed batch index * chore: add changeset * l2geth: sync spec refactor (#822) * l2geth: more in depth usage string * l2geth: add standard client getters for index * l2geth: refactor sync service into shared codepaths * l2geth: clean up tests * l2geth: better logging and error handling * test: improve test coverage around timestamps * l2geth: improve docstring * l2geth: rename variable * sync-service: better logline * l2geth: better logline * l2geth: test apply indexed transaction * l2geth: better logline * linting: fix * syncservice: fix logline * l2geth: add error and fix logline * l2geth: sync service tests * fix: get last confirmed enqueue (#846) * l2geth: fix get last confirmed enqueue * chore: add changeset * client: return error correctly * batch-submitter: updated config (#847) * batch-submitter: backwards compatible configuration * chore: add changeset * deps: update * js: move bcfg interface to core-utils * batch-submitter: parse USE_SENTRY and add to env example * chore: add changeset * batch-submitter: parse as float instead of int * batch-submitter: better error logging * l2geth: update rawdb logline Co-authored-by: Georgios Konstantopoulos <[email protected]> * l2geth: more robust testing Co-authored-by: Georgios Konstantopoulos <[email protected]> * l2geth: add sanity check for L1ToL2 timestamps * l2geth: handle error in single place * l2geth: fix test tx queue origin * l2geth: add new arg to start.sh * l2geth: return error in the SyncService.Start() * chore: go fmt Co-authored-by: Georgios Konstantopoulos <[email protected]>

tynes requested review from annieke and karlfloersch as code owners May 12, 2021 05:37

tynes changed the title ~~Bs/config update~~ batch-submitter: updated config May 12, 2021

github-actions bot added A-ts-packages A-op-batcher Area: op-batcher labels May 12, 2021

tynes force-pushed the bs/config-update branch from 3ed11d1 to 5dfa1ad Compare May 12, 2021 05:44

tynes force-pushed the bs/config-update branch from 3fafccf to 567db86 Compare May 12, 2021 06:22

tynes added 3 commits May 11, 2021 23:23

batch-submitter: backwards compatible configuration

a9fc573

chore: add changeset

419406f

deps: update

567db86

annieke suggested changes May 12, 2021

View reviewed changes

smartcontracts reviewed May 12, 2021

View reviewed changes

packages/batch-submitter/src/exec/run-batch-submitter.ts Outdated Show resolved Hide resolved

js: move bcfg interface to core-utils

dd62068

tynes force-pushed the bs/config-update branch from ca5591e to dd62068 Compare May 12, 2021 21:35

tynes requested review from ben-chain and K-Ho as code owners May 12, 2021 21:35

github-actions bot added A-pkg-core-utils Area: packages/core-utils M-dtl labels May 12, 2021

Merge branch 'develop' into bs/config-update

a27ee8b

annieke approved these changes May 12, 2021

View reviewed changes

tynes added 2 commits May 12, 2021 16:07

batch-submitter: parse USE_SENTRY and add to env example

723066e

chore: add changeset

b0496c1

tynes commented May 12, 2021

View reviewed changes

packages/batch-submitter/src/exec/run-batch-submitter.ts Outdated Show resolved Hide resolved

tynes added 2 commits May 12, 2021 16:54

batch-submitter: parse as float instead of int

a706979

Merge branch 'develop' into bs/config-update

eb377a1

annieke mentioned this pull request May 12, 2021

feat[batch-submitter]: add lightweight metrics server #792

Merged

batch-submitter: better error logging

1a45645

tynes merged commit 96a586e into develop May 13, 2021

tynes deleted the bs/config-update branch May 13, 2021 03:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

batch-submitter: updated config #847

batch-submitter: updated config #847

tynes commented May 12, 2021 •

edited

Loading

changeset-bot bot commented May 12, 2021 •

edited

Loading

codecov-commenter commented May 12, 2021 •

edited

Loading

annieke left a comment

annieke May 12, 2021

tynes May 12, 2021

annieke May 12, 2021

tynes commented May 12, 2021

tynes commented May 12, 2021

annieke left a comment

annieke May 12, 2021

tynes commented May 12, 2021

batch-submitter: updated config #847

batch-submitter: updated config #847

Conversation

tynes commented May 12, 2021 • edited Loading

changeset-bot bot commented May 12, 2021 • edited Loading

🦋 Changeset detected

codecov-commenter commented May 12, 2021 • edited Loading

Codecov Report

annieke left a comment

Choose a reason for hiding this comment

annieke May 12, 2021

Choose a reason for hiding this comment

tynes May 12, 2021

Choose a reason for hiding this comment

annieke May 12, 2021

Choose a reason for hiding this comment

tynes commented May 12, 2021

tynes commented May 12, 2021

annieke left a comment

Choose a reason for hiding this comment

annieke May 12, 2021

Choose a reason for hiding this comment

tynes commented May 12, 2021

tynes commented May 12, 2021 •

edited

Loading

changeset-bot bot commented May 12, 2021 •

edited

Loading

codecov-commenter commented May 12, 2021 •

edited

Loading