-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Indexer: Add Indexer and Hasura GraphQl to movement #295
Conversation
image: hasura/graphql-data-connector:v2.40.0 | ||
restart: always | ||
ports: | ||
- 8081:8081 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
@@ -40,15 +40,6 @@ processes: | |||
initial_delay_seconds: 3 | |||
exec: | |||
command: grpcurl -plaintext 0.0.0.0:30730 list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Goodbye
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why the test of the grpc port of the lightnode should be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to comment on the removals below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Below it's the suzuka-full-node entry. I still don't see.
I see there's a postgres entry in the suzuka-full-node/docker-compose.yml. Postgres is not needed for the Suzuka-full-node. Perhaps it should be removed and added to the indexer docker-compose or just removed. Depends on how postgres db is managed.
scripts/postgres/start-dev
Outdated
@@ -7,6 +7,10 @@ rm -rf ./.data | |||
# Initialize the database cluster | |||
initdb -D ./.data --no-locale | |||
|
|||
# allow docker connection to the db. | |||
sed -i "/listen_addresses/c\listen_addresses = \'*\'" ./.data/postgresql.conf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nvm, getting invalid command code .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sed 1: "./.data/postgresql.conf": invalid command code .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I correct Mac specific errors. I should work now on mac.
@@ -31,24 +28,23 @@ impl Executor { | |||
self.maptos_config.chain.maptos_chain_id.clone(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably add parameters to the config to turn this on and off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what to turn on/off? The chain_id is mandatory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, it looks like I didn't select the range properly. I meant the run_indexer_grpc_service
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes run_indexer_grpc_service
is only needed for the indexer. It's part of the Executor config. I don't see how to turn it off. Perhaps I should move it to a specific indexer config struct that is only defined for the indexer process?
// }; | ||
|
||
//create config file | ||
let indexer_config_content = format!( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, looks very nice! But, I need a fix to run a play around with.
I've added an e2e test. For Indexer, I query the DB to see if there's some Tx. For Hasura, I just test if the console is available. I do some search to see how to configure Hasura and declare the indexer table to be able to graphQl query the transaction table, but I didn't see an obvious way, so I test the DB and the presence of Hasura. |
|
||
indexer-test: | ||
command: | | ||
sleep 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reasonable for now.
@@ -40,15 +40,6 @@ processes: | |||
initial_delay_seconds: 3 | |||
exec: | |||
command: grpcurl -plaintext 0.0.0.0:30730 list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to comment on the removals below.
@@ -31,24 +28,23 @@ impl Executor { | |||
self.maptos_config.chain.maptos_chain_id.clone(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, it looks like I didn't select the range properly. I meant the run_indexer_grpc_service
.
ports: | ||
- "8085:8085" | ||
restart: always | ||
environment: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, does the existing Docker compose work for the rest of the indexer. That is, as long as we provide the same postgres connection here and there, are we good? Or, are we still missing the indexer-processor
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to provide 2 docker scripts, one for Hasura and one for the indexer. I can use the same Ip env variable for the indexer docker script.
@musitdev it looks like the indexer test has a legitimate error: https://github.com/movementlabsxyz/movement/actions/runs/10267756524/job/28452717854?pr=295 |
Ah yes, I didn't see. It's caused because Postgres db is not started, but I don't see why. The indexer depends on it in process-compose, but I don't see the logs of its start. I try to restart it. |
The test pass after restart. |
Summary
protocol-units
,networks
,scripts
,util
,cicd
, ormisc
.Add the indexer and Hasura GraphQl engine to process compose.
This PR depends on these one:
Changelog
Create an Indexer crate under network/suzuka.
Update process compose to run Indexer and Hasura.
Remove the indexer started with Suzuka node.
Create 2 process-compose script: indexer and Hasura
The Postgres Db is now started by the indexer and not Suzuka-node.
Testing
If there's some issue for building (dieser error) you need to clean the repo with
cargo clean
If you have the issue with fixed crate that need rustc version 1.79.0 execute this command:
cargo update -p fixed --precise 1.27.0
To run everything:
When the Hasura server is started, open a browser at this url:
http://localhost:8085/console
To add the Suzuka Tx db to Hasura:
Database name
postgresql://postgres:password@database:5432/postgres
To see all Tx in the Db.
To run the indexer e2e test:
Outstanding issues
I start only one indexer processor: dedfault_processor. I'm going to see how to start more processor to populate more data in the db.
Available processors are: