Skip to content
This repository has been archived by the owner on Jan 8, 2025. It is now read-only.

dev: add docker healthcheck in compose #338

Merged
merged 9 commits into from
Jul 31, 2023

Conversation

ftupas
Copy link
Contributor

@ftupas ftupas commented Jul 25, 2023

Time spent on this PR: 0.01

Resolves: #337, #371

Pull Request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build-related changes
  • Documentation content changes
  • Testing

What is the new behavior?

Create a side-car container that does a healthcheck on the starknet service that it is ready to accept connections before deploying Kakarot contracts

Does this introduce a breaking change?

  • Yes
  • No

docker-compose.yaml Outdated Show resolved Hide resolved
docker-compose.yaml Outdated Show resolved Hide resolved
Copy link
Member

@ClementWalter ClementWalter left a comment

Choose a reason for hiding this comment

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

lgtm, though I'd like to make sure that madara has no healthcheck route available first.

We should also add a healthcheck to katana, even though it's fast enough we actually want to make sure that we don't start the other containers until the "starknet" network is ready

@ftupas ftupas force-pushed the dev/add-healthcheck-madara branch from e1e1109 to 4232be2 Compare July 26, 2023 09:25
docker-compose.yaml Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Patch coverage: 81.12% and project coverage change: +50.07% 🎉

Comparison is base (71506a8) 23.17% compared to head (85234cd) 73.25%.
Report is 127 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #338       +/-   ##
===========================================
+ Coverage   23.17%   73.25%   +50.07%     
===========================================
  Files           9       30       +21     
  Lines        1247     2202      +955     
===========================================
+ Hits          289     1613     +1324     
+ Misses        958      589      -369     
Files Changed Coverage Δ
crates/core/src/mock/serde.rs 0.00% <0.00%> (ø)
crates/core/src/models/balance.rs 0.00% <0.00%> (ø)
crates/core/src/models/transaction.rs 93.57% <ø> (ø)
crates/eth-rpc/src/api/alchemy_api.rs 0.00% <ø> (ø)
crates/eth-rpc/src/api/eth_api.rs 0.00% <ø> (ø)
crates/eth-rpc/src/api/net_api.rs 0.00% <ø> (ø)
crates/eth-rpc/src/api/web3_api.rs 0.00% <ø> (ø)
crates/eth-rpc/src/config.rs 0.00% <ø> (ø)
crates/eth-rpc/src/lib.rs 0.00% <ø> (ø)
crates/eth-rpc/src/main.rs 0.00% <ø> (ø)
... and 20 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ftupas ftupas requested a review from ClementWalter July 27, 2023 16:19
Copy link
Member

@Eikix Eikix left a comment

Choose a reason for hiding this comment

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

Since health check got merged in dojo, what now?:)

@ftupas
Copy link
Contributor Author

ftupas commented Jul 27, 2023

Since health check got merged in dojo, what now?:)

Let me pin the image tag for dojo 👍

docker-compose.katana.yaml Outdated Show resolved Hide resolved
@Eikix
Copy link
Member

Eikix commented Jul 27, 2023

Alright great

ClementWalter
ClementWalter previously approved these changes Jul 28, 2023
@ftupas ftupas dismissed ClementWalter’s stale review July 28, 2023 07:04

The merge-base changed after approval.

ClementWalter
ClementWalter previously approved these changes Jul 28, 2023
@ftupas ftupas dismissed ClementWalter’s stale review July 28, 2023 07:10

The merge-base changed after approval.

ClementWalter
ClementWalter previously approved these changes Jul 28, 2023
@ftupas ftupas dismissed ClementWalter’s stale review July 28, 2023 07:33

The merge-base changed after approval.

ClementWalter
ClementWalter previously approved these changes Jul 28, 2023
@ftupas ftupas dismissed ClementWalter’s stale review July 28, 2023 07:43

The merge-base changed after approval.

Copy link
Member

@Eikix Eikix left a comment

Choose a reason for hiding this comment

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

lgtm

Eikix
Eikix previously approved these changes Jul 28, 2023
Copy link
Member

@Eikix Eikix left a comment

Choose a reason for hiding this comment

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

lgtm

@ftupas ftupas dismissed Eikix’s stale review July 28, 2023 07:51

The merge-base changed after approval.

docker-compose.yaml Outdated Show resolved Hide resolved
@ftupas ftupas force-pushed the dev/add-healthcheck-madara branch from b4ae6d3 to 85234cd Compare July 31, 2023 07:26
docker-compose.katana.yaml Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
@ClementWalter ClementWalter added this pull request to the merge queue Jul 31, 2023
Merged via the queue into kkrt-labs:main with commit 2be3125 Jul 31, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Aug 1, 2023
@ftupas ftupas deleted the dev/add-healthcheck-madara branch August 18, 2023 07:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

feat: add a healthcheck in docker compose
3 participants