feat(localnet): Add --no-explorer flag and reduce size #653

abukosek · 2024-11-21T11:30:18Z

This PR:

Adds the --no-explorer flag, which disables starting the explorer and indexer.
Adds the --no-gateway flag, which disables starting the gateway.
If the gateway is disabled, the explorer and indexer are as well.
Reduces the size of the sapphire-localnet Docker image from 985MB to 932MB.
Reduces the number of layers in the sapphire-localnet Docker image.
Makes it easier to build components from unreleased branches (you no longer have to uncomment whole blocks of shell scripts, just a single line).
Moves all the version variables to the top, so you don't have to dig through all the crap in the Dockerfile just to bump a version.
Bumps CLI version to 0.10.3 and Nexus version to 0.4.2.

Closes #640.

codecov · 2024-11-21T11:46:42Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.01%. Comparing base (b39df6c) to head (3ec05bd).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #653   +/-   ##
=======================================
  Coverage   59.01%   59.01%           
=======================================
  Files          40       40           
  Lines        4384     4384           
=======================================
  Hits         2587     2587           
  Misses       1574     1574           
  Partials      223      223

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

🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests

matevz

First pass, I'll test it tomorrow.

matevz · 2024-11-21T16:15:27Z

docker/sapphire-localnet/Dockerfile

+
+# Create symlinks for convenience and a dir to put all your rofls into,
+# also print version info.
+RUN ln -s ${OASIS_NODE_BINARY} ${OASIS_CLI_BINARY} /usr/local/bin \


Maybe also link the web3 gateway there for consistency?

I don't think that's useful, since the gateway is started at container startup if enabled and the user won't ever need to call it manually, whereas the CLI and node have useful commands. This is probably why the original author of these links only linked the node and CLI :)

ptrus

A couple of minor suggestions. The updates to sapphire-localnet/Dockerfile are great, much clearer what versions of components are used.

ptrus · 2024-11-22T08:34:00Z

docker/common/start.sh

@@ -18,6 +18,8 @@

 rm -f /CONTAINER_READY

+export OASIS_DOCKER_START_EXPLORER=${OASIS_DOCKER_START_EXPLORER:-yes}
+
 export OASIS_DOCKER_NO_GATEWAY=${OASIS_DOCKER_NO_GATEWAY:-no}


Should we change this to OASIS_DOCKER_START_GATEWAY, for consistency with explorer?

Also I just remembered that enabling nexus/explorer while gateway is disabled wont work, so let's maybe catch that and error out (or alternative, if gateway is disabled also automatically disable nexus/explorer).

Good catch, I'll fix that :)

Should we change this to OASIS_DOCKER_START_GATEWAY, for consistency with explorer?

I've kept this as-is for compatibility, but we can do a separate PR to change this in the future.

ptrus · 2024-11-22T08:35:33Z

docker/common/start.sh

@@ -148,6 +150,11 @@ while [[ $# -gt 0 ]]; do
      shift
      shift
      ;;
+    --no-explorer)


Should we also add a flag for disabling gateway?

I would simply disable both the explorer and indexer, if --no-explorer is passed.

I would simply disable both the explorer and indexer, if --no-explorer is passed.

That is already what happens, Peter suggested adding a similar option for the gateway :)

ptrus · 2024-11-22T08:38:00Z

docker/common/start.sh

-  su -c "createdb -h 127.0.0.1 -p 5432 -U postgres nexus" postgres
+# Once everything is initialized and setup, start Nexus and Explorer if enabled.
+if [[ "${OASIS_DOCKER_START_EXPLORER}" == "yes" ]]; then
+  if [ ! -z "${OASIS_NEXUS_BINARY:-}" ]; then


I think we can remove this check for the binary now and assume it's set if explorer is enabled? (like we do for gateway).

ptrus · 2024-11-22T08:45:29Z

docker/sapphire-localnet/Dockerfile

+    \
+    && cd / \
+    && if [ "x${OASIS_CORE_BRANCH}" != "x" ]; then \
+        git clone https://github.com/oasisprotocol/oasis-core.git --branch ${OASIS_CORE_BRANCH} --depth 1 \


Could we use a pattern that allows specifying either branch name or commit ref, and works in both cases?

I believe the clone->fetch->checkout as it is done for Explorer repository works with both. Maybe there's a more concise alternative, but it would be great if we would allow either branch name or specific commit refs to be specified for any component.

ptrus · 2024-11-22T09:02:18Z

docker/sapphire-localnet/Dockerfile

-# ARG EXPLORER_REPO=https://github.com/oasisprotocol/explorer.git
-# ARG EXPLORER_VERSION=v1.13.0
+#ARG NEXUS_BRANCH=main
+
 # TODO: Update to a released version once a version with localnet explorer support is available (https://github.com/oasisprotocol/explorer/issues/1597).
 ARG EXPLORER_REPO=https://github.com/oasisprotocol/explorer.git


Feel free to remove the variable for the repository. I used it because my (very hacky) fork was used before explorer got some real support for localnet in oasisprotocol/explorer#1597 , but then forgot to remove it 😁

matevz

While you're at it, can you also move the explorer port 80 to 8548 (see the last bullet #640) and the docs accordingly.

matevz · 2024-11-22T10:40:59Z

docker/common/start.sh

@@ -148,6 +150,11 @@ while [[ $# -gt 0 ]]; do
      shift
      shift
      ;;
+    --no-explorer)


I would simply disable both the explorer and indexer, if --no-explorer is passed.

matevz · 2024-11-22T10:42:56Z

.github/workflows/ci-benchmarks.yml

@@ -44,6 +44,7 @@ jobs:
      SAPPHIRE_PARATIME: ${{ github.workspace }}/oasis_core/sapphire-paratime
      OASIS_NODE_DATADIR: /tmp/oasis-sapphire-benchmarks
      OASIS_WEB3_GATEWAY: ${{ github.workspace }}/oasis-web3-gateway
+      OASIS_DOCKER_START_EXPLORER: no


Don't forget to describe the usage in the Sapphire Localnet chapter: https://github.com/oasisprotocol/docs/blob/main/docs/dapp/tools/localnet.mdx

PR opened in docs: oasisprotocol/docs#1032

abukosek · 2024-11-22T11:20:38Z

While you're at it, can you also move the explorer port 80 to 8548 (see the last bullet #640) and the docs accordingly.

The port move seems to have already been done in a941da4

abukosek · 2024-11-25T13:20:10Z

Ready for re-review :)

In a future PR, we can replace nodejs with nginx and save about 200MB+.

abukosek force-pushed the andrej/feature/no-explorer branch 2 times, most recently from 7a30d81 to 3ec05bd Compare November 21, 2024 11:41

abukosek marked this pull request as ready for review November 21, 2024 11:54

abukosek requested review from kostko and ptrus as code owners November 21, 2024 11:54

abukosek requested review from matevz and removed request for kostko and ptrus November 21, 2024 11:54

matevz reviewed Nov 21, 2024

View reviewed changes

ptrus reviewed Nov 22, 2024

View reviewed changes

matevz reviewed Nov 22, 2024

View reviewed changes

feat(localnet): Add --no-explorer flag and reduce size

d39e59d

abukosek force-pushed the andrej/feature/no-explorer branch from 3ec05bd to d39e59d Compare November 25, 2024 12:56

abukosek mentioned this pull request Nov 25, 2024

docs: Update localnet docs oasisprotocol/docs#1032

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(localnet): Add --no-explorer flag and reduce size #653

feat(localnet): Add --no-explorer flag and reduce size #653

abukosek commented Nov 21, 2024 •

edited

Loading

codecov bot commented Nov 21, 2024

matevz left a comment

matevz Nov 21, 2024

abukosek Nov 22, 2024

ptrus left a comment

ptrus Nov 22, 2024

ptrus Nov 22, 2024

abukosek Nov 22, 2024

abukosek Nov 25, 2024

ptrus Nov 22, 2024

matevz Nov 22, 2024 •

edited

Loading

abukosek Nov 22, 2024

ptrus Nov 22, 2024

ptrus Nov 22, 2024

ptrus Nov 22, 2024

abukosek Nov 22, 2024

matevz left a comment

matevz Nov 22, 2024 •

edited

Loading

matevz Nov 22, 2024

abukosek Nov 25, 2024

abukosek commented Nov 22, 2024

abukosek commented Nov 25, 2024

feat(localnet): Add --no-explorer flag and reduce size #653

Are you sure you want to change the base?

feat(localnet): Add --no-explorer flag and reduce size #653

Conversation

abukosek commented Nov 21, 2024 • edited Loading

codecov bot commented Nov 21, 2024

Codecov Report

matevz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ptrus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matevz Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matevz left a comment

Choose a reason for hiding this comment

matevz Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abukosek commented Nov 22, 2024

abukosek commented Nov 25, 2024

abukosek commented Nov 21, 2024 •

edited

Loading

matevz Nov 22, 2024 •

edited

Loading

matevz Nov 22, 2024 •

edited

Loading