Update positions checks - #715
Conversation
Signed-off-by: cyc60 <avsysoev60@gmail.com>
Signed-off-by: cyc60 <avsysoev60@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the update-redeemable-positions internal command to run additional pre-flight checks (execution nodes, graph sync, IPFS upload) and to base subgraph queries on the finalized block, while adjusting tests to bypass the new startup checks.
Changes:
- Add a
_startup_check()flow before processing, including execution node, network, graph sync, Arbitrum (mainnet), and IPFS upload checks. - Switch position fetching to use the finalized block number instead of the latest block number.
- Update internal CLI tests to patch out the new startup check.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/commands/tests/test_internal/test_update_redeemable_positions.py |
Patches the new startup check in CLI tests so existing test scenarios can continue to run in isolation. |
src/commands/internal/update_redeemable_positions.py |
Adds pre-flight checks, changes block selection to finalized, removes confirmation/no-confirm flow, and introduces IPFS/Arbitrum connectivity helpers. |
Comments suppressed due to low confidence (1)
src/commands/internal/update_redeemable_positions.py:160
- The PR removes the interactive confirmation step and also drops the
--no-confirmflag for this CLI. This is a breaking CLI/API behavior change and also removes a safety guard before publishing new data to IPFS. Consider restoring a confirmation prompt by default and keeping--no-confirm(or an equivalent--yes) for automation/backwards compatibility, rather than making uploads always non-interactive.
@click.command(help='Updates redeemable os token positions')
# pylint: disable-next=too-many-arguments,too-many-locals
def update_redeemable_positions(
execution_endpoints: str,
execution_jwt_secret: str | None,
graph_endpoint: str,
arbitrum_endpoint: str | None,
network: str,
verbose: bool,
log_level: str,
min_os_token_position_amount_gwei: int,
api_sleep_timeout: float,
api_source: str,
api_access_key: str | None,
) -> None:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| setup_logging() | ||
| await setup_clients() | ||
| await _startup_check(arbitrum_config) | ||
| try: |
There was a problem hiding this comment.
_startup_check() calls wait_for_execution_node() / wait_for_graph_node_sync_to_chain_head(), both of which retry indefinitely. For a one-off/batch command like update_redeemable_positions, this can cause the command to hang forever on misconfiguration/outages. Consider adding a bounded timeout/max-retries for these waits (and surfacing a clear error) so the command fails fast when dependencies are unavailable.
| ipfs_upload_client = build_ipfs_upload_clients() | ||
| ipfs_hash = await ipfs_upload_client.upload_json({'a': 'b'}) | ||
| logger.info('Connected to IPFS upload clients. Test hash: %s', ipfs_hash) |
There was a problem hiding this comment.
_check_ipfs_upload_clients() performs a real upload_json({'a': 'b'}) as a connectivity check. This likely creates/pins junk content on every run (and does so even if there end up being no positions to upload), which has cost/operational implications for Pinata/Infura/local nodes. Prefer a non-mutating health check (e.g., validate client configuration, call a lightweight status endpoint if available, or defer the upload check until right before the real upload and reuse the actual payload/hash).
| ipfs_upload_client = build_ipfs_upload_clients() | |
| ipfs_hash = await ipfs_upload_client.upload_json({'a': 'b'}) | |
| logger.info('Connected to IPFS upload clients. Test hash: %s', ipfs_hash) | |
| # Avoid performing a real upload/pin during startup checks. Building the | |
| # client validates configuration and initialization without mutating remote | |
| # IPFS state or creating junk test content. | |
| build_ipfs_upload_clients() | |
| logger.info('IPFS upload clients are configured and initialized.') |
No description provided.