Skip to content

feat(BaseServiceV2):End immediately when looping#2731

Merged
mergify[bot] merged 9 commits intodevelopfrom
willc/end-loop-on-stop
Jun 14, 2022
Merged

feat(BaseServiceV2):End immediately when looping#2731
mergify[bot] merged 9 commits intodevelopfrom
willc/end-loop-on-stop

Conversation

@roninjin10
Copy link
Copy Markdown
Contributor

@roninjin10 roninjin10 commented Jun 9, 2022

The server does not end when we call stop because async loops do not end. This makes it so the async loop stops right away.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Jun 9, 2022 •
edited
Loading

🦋 Changeset detected

Latest commit: 1aae746

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

This PR includes changesets to release 6 packages
Name Type
@eth-optimism/common-ts Minor
@eth-optimism/data-transport-layer Patch
@eth-optimism/drippie-mon Patch
@eth-optimism/fault-detector Patch
@eth-optimism/message-relayer Patch
@eth-optimism/replica-healthcheck 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

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 9, 2022

This PR changes implementation code, but doesn't include a changeset. Did you forget to add one?

@roninjin10 roninjin10 force-pushed the willc/end-loop-on-stop branch 2 times, most recently from 2225770 to ea76a7b Compare June 9, 2022 05:12
@roninjin10 roninjin10 changed the title End immediately when looping feat(BaseServiceV2):End immediately when looping Jun 9, 2022
@roninjin10 roninjin10 force-pushed the willc/end-loop-on-stop branch from ea76a7b to ddd6279 Compare June 9, 2022 17:17
@roninjin10
Copy link
Copy Markdown
Contributor Author

roninjin10 commented Jun 9, 2022

Added do not merge because this should work but I haven't actually ran the code yet with latest changes

smartcontracts
smartcontracts reviewed Jun 9, 2022
View reviewed changes
@roninjin10 roninjin10 force-pushed the willc/end-loop-on-stop branch from 4c8fd74 to f40f179 Compare June 9, 2022 19:31
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 9, 2022

Hey @roninjin10! This PR has merge conflicts. Please fix them before continuing review.

@mergify mergify bot added the conflict label Jun 9, 2022
@roninjin10 roninjin10 force-pushed the willc/end-loop-on-stop branch from f40f179 to 060676a Compare June 9, 2022 20:02
@mergify mergify bot removed the conflict label Jun 9, 2022
tynes
tynes reviewed Jun 9, 2022
View reviewed changes
@roninjin10 roninjin10 force-pushed the willc/end-loop-on-stop branch from f59c504 to 18e6fe8 Compare June 9, 2022 20:18
smartcontracts
smartcontracts requested changes Jun 9, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@smartcontracts smartcontracts left a comment

Choose a reason for hiding this comment

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

I liked the old behavior of this.running better

@roninjin10 roninjin10 force-pushed the willc/end-loop-on-stop branch from 058fdaf to 3511a72 Compare June 9, 2022 22:25
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 9, 2022

Hey @roninjin10! This PR has merge conflicts. Please fix them before continuing review.

@mergify mergify bot added the conflict label Jun 9, 2022
@github-actions github-actions bot added 2-reviewers C-protocol-critical Category: Modifies protocol-critical code A-cannon Area: cannon labels Jun 9, 2022
@roninjin10 roninjin10 force-pushed the willc/end-loop-on-stop branch from 3511a72 to 9e2a2a3 Compare June 9, 2022 22:27
smartcontracts
smartcontracts reviewed Jun 13, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@smartcontracts smartcontracts left a comment

Choose a reason for hiding this comment

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

Small typo, otherwise lgtm. Let's fix that then can approve and merge.

@roninjin10 roninjin10 force-pushed the willc/end-loop-on-stop branch 3 times, most recently from b9bc742 to 644d064 Compare June 14, 2022 18:21
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 14, 2022

This PR has been added to the merge queue, and will be merged soon.

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 14, 2022

This PR is next in line to be merged, and will be merged as soon as checks pass.

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 14, 2022

Merge failed. Please see automated check logs for more details.

@mergify mergify bot removed the on-merge-train label Jun 14, 2022
@roninjin10 roninjin10 force-pushed the willc/end-loop-on-stop branch from 58af274 to 1aae746 Compare June 14, 2022 21:13
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 14, 2022

This PR has been added to the merge queue, and will be merged soon.

@mergify mergify bot merged commit 52b2687 into develop Jun 14, 2022
@mergify mergify bot deleted the willc/end-loop-on-stop branch June 14, 2022 21:29
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 14, 2022

This PR is next in line to be merged, and will be merged as soon as checks pass.

@mergify mergify bot removed the on-merge-train label Jun 14, 2022
@mslipper mslipper mentioned this pull request Jun 18, 2022
Merged
theochap added a commit that referenced this pull request Dec 10, 2025
@theochap
feat(node/tests): fully integrate sysgo into testing framework + CI (#…
3d21e87 
…2731)

## Description

This PR integrates sysgo inside our testing framework and adds a CI job
to ensure all our tests pass with sysgo enabled

Reorganizes the e2e test folders. `common` contains tests to run both in
sysgo and kurtosis. There are a few kurtosis tests we cannot run in
sysgo yet such as:
- CPU monitoring tests (doesn't make sense for in-memory mode)
- Custom dev websocket tests (cannot access to custom websocket
subscriptions through op-node devstack API)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tynes tynes tynes approved these changes

@smartcontracts smartcontracts smartcontracts approved these changes

@mslipper mslipper Awaiting requested review from mslipper

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@roninjin10 @tynes @smartcontracts