Skip to content

batch-submitter: updated config#847

Merged
tynes merged 10 commits intodevelopfrom
bs/config-update
May 13, 2021
Merged

batch-submitter: updated config#847
tynes merged 10 commits intodevelopfrom
bs/config-update

Conversation

@tynes
Copy link
Copy Markdown
Contributor

@tynes tynes commented May 12, 2021

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
Copy link
Copy Markdown

changeset-bot bot commented May 12, 2021

🦋 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

@tynes tynes changed the title Bs/config update batch-submitter: updated config May 12, 2021
@tynes tynes force-pushed the bs/config-update branch from 3ed11d1 to 5dfa1ad Compare May 12, 2021 05:44
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 12, 2021

Codecov Report

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

Impacted file tree graph

@@           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.

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

@annieke annieke left a comment

Choose a reason for hiding this comment

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

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

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.

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

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.

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

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.

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

@tynes
Copy link
Copy Markdown
Contributor Author

tynes commented May 12, 2021

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 tynes force-pushed the bs/config-update branch from ca5591e to dd62068 Compare May 12, 2021 21:35
@tynes tynes requested review from K-Ho and ben-chain as code owners May 12, 2021 21:35
@github-actions github-actions bot added A-pkg-core-utils Area: packages/core-utils M-dtl labels May 12, 2021
@tynes
Copy link
Copy Markdown
Contributor Author

tynes commented May 12, 2021

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

Copy link
Copy Markdown
Contributor

@annieke annieke left a comment

Choose a reason for hiding this comment

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

lgtm with one nit

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.

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

@tynes
Copy link
Copy Markdown
Contributor Author

tynes commented May 12, 2021

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

@tynes tynes merged commit 96a586e into develop May 13, 2021
@tynes tynes deleted the bs/config-update branch May 13, 2021 03:56
tynes added a commit that referenced this pull request May 13, 2021
* 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
InoMurko pushed a commit to omgnetwork/optimism that referenced this pull request May 25, 2021
* 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
tynes added a commit that referenced this pull request May 26, 2021
…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 <me@gakonst.com>

* l2geth: more robust testing

Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>

* 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 <me@gakonst.com>
theochap pushed a commit that referenced this pull request Dec 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-op-batcher Area: op-batcher A-pkg-core-utils Area: packages/core-utils

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants