Skip to content

Conversation

@halfdoctor
Copy link
Contributor

This pull request updates the build scripts.

@halfdoctor
Copy link
Contributor Author

Summary of Changes:

The main purpose of these changes is to make the process of passing the environment file path from the configure-environment.sh script (running inside a Docker container) back to the build.sh host script more robust, flexible, and reliable in Docker environments, especially when running multiple builds or in parallel.


What exactly changed?

1. Temporary File Handling

  • Before:
    The build.sh script used a single temporary file (mktemp) to pass the result from the container via a bind mount (-v "$SETUP_RESULT_FILE:/tmp/setup_result").
    The container would write the environment file path to /tmp/setup_result, which mapped to the host's temp file.

  • After:
    Now, a unique temporary directory is created on the host using mktemp -d, and the result file's path is constructed inside it ($SETUP_RESULT_DIR/setup_result).
    This entire directory is mounted into the container (-v "$SETUP_RESULT_DIR:/tmp/setup_result_dir"), and the container writes to /tmp/setup_result_dir/setup_result.

2. Result File Passing

  • The CLI argument --result-file <path> is introduced and passed to configure-environment.sh only when running in Docker mode.
  • The script now writes the result file to the path specified by this argument rather than assuming /tmp/setup_result.

3. Cleanup

  • Before:
    Only the temporary file was deleted (rm -f "$SETUP_RESULT_FILE").
  • After:
    The entire temporary directory is deleted (rm -rf "$SETUP_RESULT_DIR"), ensuring all artifacts are cleaned up.

4. Optional Variable Added

  • The environment variable TELEGRAM_MESSAGE_THREAD_ID is added to the list of optional variables.
    This is unrelated to the temp file changes and seems to be a feature addition.

Why are these changes necessary?

Problems with the Old Approach

  1. File vs. Directory Bind Mounting

    • Mounting a single file (-v $HOST_FILE:$CONTAINER_FILE) has limitations in Docker, especially if the container tries to create or move files.
    • If multiple builds are run in parallel, there's a risk of conflicts (same filename), or stale files could be reused accidentally.
  2. Robustness

    • Using a directory for temp results is more robust: Docker bind-mounting a directory is safer, less error-prone, and makes it easier to clean up.
    • Passing the result file path as an argument to the script (--result-file) makes the script more flexible and decoupled from assumptions about the environment.
  3. Reliability

    • The script now explicitly checks and writes to the path it's told to, ensuring no accidental overwrites or permission issues.
    • Cleaning up the entire directory ensures no stray files are left behind.
  4. Extensibility

    • By adopting a result directory and a CLI argument, it's easier to extend this pattern in the future (e.g., multiple result files, logs, etc.).

In Summary

These changes:

  • Make the result handoff between container and host more reliable and Docker-friendly.
  • Prevent file conflicts and permission issues, especially in concurrent or CI environments.
  • Clean up after themselves more thoroughly.
  • Add a minor feature (TELEGRAM_MESSAGE_THREAD_ID) for new functionality.

In one sentence:
The changes make the build and environment configuration process more robust, flexible, and reliable by using directory-based temp storage and explicit file passing between host and Docker container.

@halfdoctor halfdoctor requested a review from anomit September 2, 2025 09:05
Copy link
Member

@anomit anomit left a comment

Choose a reason for hiding this comment

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

Successfully deployed multiple slots by deploying the experimental build of lite node, with Snapshotter CLI.

Investigated the slot launch screen for logs, and everything seems okay

ℹ️ Auto-selected existing environment file: ./.env-mainnet-UNISWAPV2-ETH
🟢 Using environment file: ./.env-mainnet-UNISWAPV2-ETH
🟢 Operating on selected environment file: ./.env-mainnet-UNISWAPV2-ETH
🔔 ./.env-mainnet-UNISWAPV2-ETH has OVERRIDE_DEFAULTS=true. Preserving existing overrides.
🔍 Overriding POWERLOOM_CHAIN in ./.env-mainnet-UNISWAPV2-ETH with value: mainnet (existing value: MAINNET)
🔍 Adding new variable SNAPSHOT_CONFIG_REPO_COMMIT to ./.env-mainnet-UNISWAPV2-ETH with value: cc740e46687ecd6732e11bd59fa7918db82099f5
🔍 Adding new variable SNAPSHOTTER_COMPUTE_REPO_COMMIT to ./.env-mainnet-UNISWAPV2-ETH with value: ebb5bbf66122de45fae10c9ef908102e010a8b24
🔍 Skipping update for CONNECTION_REFRESH_INTERVAL_SEC, using existing value: 75
🔍 Skipping update for TELEGRAM_NOTIFICATION_COOLDOWN, using existing value: 300
🔔 Skipping credential update prompts due to --skip-credential-update flag
🔔 LOCAL_COLLECTOR_PORT not found in ./.env-mainnet-UNISWAPV2-ETH, setting to default value 50051 and adding to file.
🔔 MAX_STREAM_POOL_SIZE not found in ./.env-mainnet-UNISWAPV2-ETH, setting to default value 2 and adding to file.
🔍 Overriding MAX_STREAM_POOL_SIZE in ./.env-mainnet-UNISWAPV2-ETH with value: 2 (existing value: 10)
🔔 STREAM_HEALTH_CHECK_TIMEOUT_MS not found in ./.env-mainnet-UNISWAPV2-ETH, setting to default value 5000 and adding to file.
🔍 Adding new variable STREAM_HEALTH_CHECK_TIMEOUT_MS to ./.env-mainnet-UNISWAPV2-ETH with value: 5000
🔔 STREAM_WRITE_TIMEOUT_MS not found in ./.env-mainnet-UNISWAPV2-ETH, setting to default value 5000 and adding to file.
🔍 Adding new variable STREAM_WRITE_TIMEOUT_MS to ./.env-mainnet-UNISWAPV2-ETH with value: 5000
🔔 MAX_WRITE_RETRIES not found in ./.env-mainnet-UNISWAPV2-ETH, setting to default value 3 and adding to file.
🔍 Adding new variable MAX_WRITE_RETRIES to ./.env-mainnet-UNISWAPV2-ETH with value: 3
🔔 MAX_CONCURRENT_WRITES not found in ./.env-mainnet-UNISWAPV2-ETH, setting to default value 4 and adding to file.
🔍 Adding new variable MAX_CONCURRENT_WRITES to ./.env-mainnet-UNISWAPV2-ETH with value: 4
🔔 TELEGRAM_MESSAGE_THREAD_ID not found in ./.env-mainnet-UNISWAPV2-ETH, setting to default value  and adding to file.
🔍 Adding new variable TELEGRAM_MESSAGE_THREAD_ID to ./.env-mainnet-UNISWAPV2-ETH with value:
✅ Configuration complete. Environment file ready at ./.env-mainnet-UNISWAPV2-ETH
🔗 Reported env file to build script: ./.env-mainnet-UNISWAPV2-ETH
Untagged: snapshotter-lite-setup:latest
Deleted: sha256:0f6f474ddbe4c3d1f1555831399783db4c618a2621afa9c67bf1f3091d91b14d
ℹ️ Setup container configured: ./.env-mainnet-UNISWAPV2-ETH
🔔 LOCAL_COLLECTOR_IMAGE_TAG found in .env, using value latest
🏗️ Running snapshotter-lite-v2 node Docker image with tag experimental

Copy link
Member

@SwaroopH SwaroopH left a comment

Choose a reason for hiding this comment

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

LGTM - confirmed with noe1 on Discord who was facing the permission issue

https://discord.com/channels/777248105636560948/1063022869040353300/1412364828118618142

@SwaroopH
Copy link
Member

SwaroopH commented Sep 3, 2025

@halfdoctor Thank you for the contribution! Commits to all our repos require verified signatures. I am going to merge this for now but please do set it up for future PRs.

https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification

@SwaroopH SwaroopH merged commit 0b0f29d into powerloom:main Sep 3, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants