fix(spo): apply review comments and coding style to ProtoFire SPO code#661
Conversation
- Replace println!/eprintln! with log::{debug, error, info, warn}
- Replace .unwrap() with .expect() with meaningful messages
- Add SIGTERM handling to spo-indexer (graceful shutdown)
- Fix telemetry service_name: "chain-indexer" → "spo-indexer"
- Remove Blockfrost ID from config.yaml (use env var instead)
- Consolidate SPO SQL migrations into 003_spo.sql
- Remove unused imports in spo-indexer
Apply coding conventions: lowercase logs, inline format args, turbofish, to_owned(), doc comment periods, and imports at file top.
Add spo-api-cli binary with print-api-schema-v1 command and justfile target to generate spo-api/graphql/schema-v1.graphql.
|
Can we skip the extra API and move the code from spo-api into indexer-api? |
Apply coding style fixes including SIGTERM casing, typed APIs, proper error handling, and import patterns. Consolidate SPO migrations into 001_initial.sql.
|
* refactor(spo): merge spo-api into indexer-api Move SPO GraphQL queries from standalone spo-api crate into indexer-api, following the existing storage trait abstraction pattern. This consolidates the API surface and eliminates the need for a separate SPO API service. Changes: - Add SPO domain types (SpoIdentity, PoolMetadata, EpochPerf, etc.) - Add SpoStorage trait with 18 storage methods - Implement SpoStorage for PostgreSQL (cloud) and SQLite (standalone) - Add SpoQuery GraphQL resolver with 20 query methods - Use MergedObject to combine Query and SpoQuery - Remove spo-api from workspace members - Update GraphQL schema with SPO queries * refactor(spo): fold SpoQuery into Query Move all SPO query methods from separate SpoQuery struct into the main Query struct, eliminating the need for MergedObject. This simplifies the GraphQL schema structure as suggested in PR review.
Done. Merged spo-api into indexer-api and folded all SPO queries into Query. |
| UNIQUE (spo_sk, epoch_no) | ||
| ); | ||
|
|
||
| -- Update "updated_at" field each time the record is updated |
There was a problem hiding this comment.
Uh! I don't like "code" in the database. Would our other databases support the same? Could we move setting the updated_at to the application logic? Maybe not for this PR, but let's at least create a ticket to improve this later.
There was a problem hiding this comment.
This trigger was in the original ProtoFire implementation. Agreed it's not ideal. I'll create a ticket to move updated_at logic to application layer. SQLite doesn't support triggers the same way, so this would need rework anyway for standalone mode.
There was a problem hiding this comment.
I have created a ticket: https://shielded.atlassian.net/browse/PM-21550
|
I noticed the SPO tables are only in the PostgreSQL migration - the original ProtoFire code didn't include SQLite migrations either. However, the SPO queries are now in indexer-api which is used by both cloud and standalone modes. Should we:
which option do you think most feasible? |
5826855 to
341d549
Compare
Whichever is the least effort for now and makes sure we still can build standalone. WDYT? |
hseeberger
left a comment
There was a problem hiding this comment.
I believe quality now is good enough; but we need to further work on the codebase in the future.
hseeberger
left a comment
There was a problem hiding this comment.
We cannot merge this yet, because of feature freeze for the next release.
|
Converted to draft, because we cannot merge it yet. |
I think adding SPO tables to the SQLite migration is the least effort. The tables will be empty in standalone mode (no spo-indexer), but queries will work without SQL errors. What do you think? |
- Delete spo-api/ directory (merged into indexer-api in PR #674) - Delete outdated MIGRATION.md (ProtoFire migration docs) - Remove spo-api service from docker-compose.yaml - Add SPO tables to SQLite migration for standalone mode
b2558f6
into
feat/ledger7-protofire-integrate-spo-services-without-sean-changes
#661) * fix: update reqwest feature from rustls-tls to rustls for reqwest 0.13 compatibility * refactor(spo): apply PR #526 review comments - Replace println!/eprintln! with log::{debug, error, info, warn} - Replace .unwrap() with .expect() with meaningful messages - Add SIGTERM handling to spo-indexer (graceful shutdown) - Fix telemetry service_name: "chain-indexer" → "spo-indexer" - Remove Blockfrost ID from config.yaml (use env var instead) - Consolidate SPO SQL migrations into 003_spo.sql - Remove unused imports in spo-indexer * style(spo): apply indexer code style patterns Apply coding conventions: lowercase logs, inline format args, turbofish, to_owned(), doc comment periods, and imports at file top. * feat(spo-api): add GraphQL schema generation CLI Add spo-api-cli binary with print-api-schema-v1 command and justfile target to generate spo-api/graphql/schema-v1.graphql. * refactor(spo): address PR #661 review comments Apply coding style fixes including SIGTERM casing, typed APIs, proper error handling, and import patterns. Consolidate SPO migrations into 001_initial.sql. * refactor(spo): merge spo-api into indexer-api (#674) * refactor(spo): merge spo-api into indexer-api Move SPO GraphQL queries from standalone spo-api crate into indexer-api, following the existing storage trait abstraction pattern. This consolidates the API surface and eliminates the need for a separate SPO API service. Changes: - Add SPO domain types (SpoIdentity, PoolMetadata, EpochPerf, etc.) - Add SpoStorage trait with 18 storage methods - Implement SpoStorage for PostgreSQL (cloud) and SQLite (standalone) - Add SpoQuery GraphQL resolver with 20 query methods - Use MergedObject to combine Query and SpoQuery - Remove spo-api from workspace members - Update GraphQL schema with SPO queries * refactor(spo): fold SpoQuery into Query Move all SPO query methods from separate SpoQuery struct into the main Query struct, eliminating the need for MergedObject. This simplifies the GraphQL schema structure as suggested in PR review. * refactor(spo): address PR #661 review comments (2nd round) * refactor(spo): delete spo-api crate and add SQLite SPO tables - Delete spo-api/ directory (merged into indexer-api in PR #674) - Delete outdated MIGRATION.md (ProtoFire migration docs) - Remove spo-api service from docker-compose.yaml - Add SPO tables to SQLite migration for standalone mode
#661) * fix: update reqwest feature from rustls-tls to rustls for reqwest 0.13 compatibility * refactor(spo): apply PR #526 review comments - Replace println!/eprintln! with log::{debug, error, info, warn} - Replace .unwrap() with .expect() with meaningful messages - Add SIGTERM handling to spo-indexer (graceful shutdown) - Fix telemetry service_name: "chain-indexer" → "spo-indexer" - Remove Blockfrost ID from config.yaml (use env var instead) - Consolidate SPO SQL migrations into 003_spo.sql - Remove unused imports in spo-indexer * style(spo): apply indexer code style patterns Apply coding conventions: lowercase logs, inline format args, turbofish, to_owned(), doc comment periods, and imports at file top. * feat(spo-api): add GraphQL schema generation CLI Add spo-api-cli binary with print-api-schema-v1 command and justfile target to generate spo-api/graphql/schema-v1.graphql. * refactor(spo): address PR #661 review comments Apply coding style fixes including SIGTERM casing, typed APIs, proper error handling, and import patterns. Consolidate SPO migrations into 001_initial.sql. * refactor(spo): merge spo-api into indexer-api (#674) * refactor(spo): merge spo-api into indexer-api Move SPO GraphQL queries from standalone spo-api crate into indexer-api, following the existing storage trait abstraction pattern. This consolidates the API surface and eliminates the need for a separate SPO API service. Changes: - Add SPO domain types (SpoIdentity, PoolMetadata, EpochPerf, etc.) - Add SpoStorage trait with 18 storage methods - Implement SpoStorage for PostgreSQL (cloud) and SQLite (standalone) - Add SpoQuery GraphQL resolver with 20 query methods - Use MergedObject to combine Query and SpoQuery - Remove spo-api from workspace members - Update GraphQL schema with SPO queries * refactor(spo): fold SpoQuery into Query Move all SPO query methods from separate SpoQuery struct into the main Query struct, eliminating the need for MergedObject. This simplifies the GraphQL schema structure as suggested in PR review. * refactor(spo): address PR #661 review comments (2nd round) * refactor(spo): delete spo-api crate and add SQLite SPO tables - Delete spo-api/ directory (merged into indexer-api in PR #674) - Delete outdated MIGRATION.md (ProtoFire migration docs) - Remove spo-api service from docker-compose.yaml - Add SPO tables to SQLite migration for standalone mode
* feat(spo): integrate spo indexer Originally authored by caparrosm (ProtoFire). Commit was unsigned and blocked PR merge due to repo requiring verified signatures. Amended author and re-signed to unblock. * fix(spo): apply review comments and coding style to ProtoFire SPO code (#661) * fix: update reqwest feature from rustls-tls to rustls for reqwest 0.13 compatibility * refactor(spo): apply PR #526 review comments - Replace println!/eprintln! with log::{debug, error, info, warn} - Replace .unwrap() with .expect() with meaningful messages - Add SIGTERM handling to spo-indexer (graceful shutdown) - Fix telemetry service_name: "chain-indexer" → "spo-indexer" - Remove Blockfrost ID from config.yaml (use env var instead) - Consolidate SPO SQL migrations into 003_spo.sql - Remove unused imports in spo-indexer * style(spo): apply indexer code style patterns Apply coding conventions: lowercase logs, inline format args, turbofish, to_owned(), doc comment periods, and imports at file top. * feat(spo-api): add GraphQL schema generation CLI Add spo-api-cli binary with print-api-schema-v1 command and justfile target to generate spo-api/graphql/schema-v1.graphql. * refactor(spo): address PR #661 review comments Apply coding style fixes including SIGTERM casing, typed APIs, proper error handling, and import patterns. Consolidate SPO migrations into 001_initial.sql. * refactor(spo): merge spo-api into indexer-api (#674) * refactor(spo): merge spo-api into indexer-api Move SPO GraphQL queries from standalone spo-api crate into indexer-api, following the existing storage trait abstraction pattern. This consolidates the API surface and eliminates the need for a separate SPO API service. Changes: - Add SPO domain types (SpoIdentity, PoolMetadata, EpochPerf, etc.) - Add SpoStorage trait with 18 storage methods - Implement SpoStorage for PostgreSQL (cloud) and SQLite (standalone) - Add SpoQuery GraphQL resolver with 20 query methods - Use MergedObject to combine Query and SpoQuery - Remove spo-api from workspace members - Update GraphQL schema with SPO queries * refactor(spo): fold SpoQuery into Query Move all SPO query methods from separate SpoQuery struct into the main Query struct, eliminating the need for MergedObject. This simplifies the GraphQL schema structure as suggested in PR review. * refactor(spo): address PR #661 review comments (2nd round) * refactor(spo): delete spo-api crate and add SQLite SPO tables - Delete spo-api/ directory (merged into indexer-api in PR #674) - Delete outdated MIGRATION.md (ProtoFire migration docs) - Remove spo-api service from docker-compose.yaml - Add SPO tables to SQLite migration for standalone mode * refactor(spo): move updated_at logic from DB trigger to application layer (PM-21550) (#725) * fix(spo): replace deprecated sidechain_getAriadneParameters RPC (#727) Use systemParameters_getAriadneParameters which sources the D Parameter from the on-chain pallet instead of Cardano. * refactor(spo): remove static metadata and OnlineClient from spo-indexer (#731) * refactor(spo): remove static metadata and OnlineClient from spo-indexer Delete ProtoFire's polkadot_metadata.scale, subxt macro, and build.rs. Replace get_block_timestamp with a DB query on the blocks table (populated by chain-indexer) and rewrite get_epoch_duration to use LegacyRpcMethods instead of OnlineClient. * feat(spo): add run-spo-indexer command to justfile * style(spo): apply rustfmt formatting * refactor: apply review feedback on get_block_timestamp * fix(spo): remove MIN_COMMITTEE_SIZE retry loop from get_committee The infinite retry loop waited for >= 300 committee members before proceeding, causing the spo-indexer to hang on all non-mainnet environments where committee sizes are much smaller (e.g. 6-10). * fix(spo): align Dockerfile with other indexer components * ci: add spo-indexer to Docker image build matrix (#744) * update Cargo.lock
Summary
Context
This PR shows changes made on top of ProtoFire's SPO indexer code.
Base branch contains main branch codes on top of the ProtoFire's original code.
Changes
Test