-
Notifications
You must be signed in to change notification settings - Fork 1k
Feature/graphman test run #3079
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
Changes from all commits
d6cb2b6
63b1e75
cd0f00c
78d1dcc
3c247c1
855c4b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -144,6 +144,17 @@ pub enum Command { | |
/// The deployments to rewind | ||
names: Vec<String>, | ||
}, | ||
/// Deploy and run an arbitrary subgraph, up to a certain block (for dev and testing purposes) -- WARNING: WILL RUN MIGRATIONS ON THE DB, DO NOT USE IN PRODUCTION | ||
Run { | ||
/// Network name (must fit one of the chain) | ||
network_name: String, | ||
|
||
/// Subgraph in the form `<IPFS Hash>` or `<name>:<IPFS Hash>` | ||
subgraph: String, | ||
|
||
/// Highest block number to process before stopping (inclusive) | ||
stop_block: i32, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs to say that the latest entities we'll have in the database will be for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not needed if my previous claim is right 😃 |
||
}, | ||
/// Check and interrogate the configuration | ||
/// | ||
/// Print information about a configuration file without | ||
|
@@ -374,6 +385,18 @@ impl Context { | |
} | ||
} | ||
|
||
fn metrics_registry(&self) -> Arc<MetricsRegistry> { | ||
self.registry.clone() | ||
} | ||
|
||
fn config(&self) -> Cfg { | ||
self.config.clone() | ||
} | ||
|
||
fn node_id(&self) -> NodeId { | ||
self.node_id.clone() | ||
} | ||
|
||
fn primary_pool(self) -> ConnectionPool { | ||
let primary = self.config.primary_store(); | ||
let pool = StoreBuilder::main_pool( | ||
|
@@ -411,6 +434,10 @@ impl Context { | |
pools | ||
} | ||
|
||
async fn store_builder(self) -> StoreBuilder { | ||
StoreBuilder::new(&self.logger, &self.node_id, &self.config, self.registry).await | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Heh .. the whole point of this Also, I would change this method to call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We changed it exactly to be able to run ... migrations :D I notice the differences indeed. Migrations was required first for CI since it always starts fresh but it's also quite good when using it for development purposes as I often starts from a fresh database. The best will be to discuss that over a video chat to see what you be the best course of action here. Two things come to mind:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lutter Need to chat more about this, indeed, we are using this command mostly on a completely empty database (in dev/test flow), so we need all the migrations bells and whistles... |
||
|
||
fn store_and_pools(self) -> (Arc<Store>, HashMap<Shard, ConnectionPool>) { | ||
let (subgraph_store, pools) = StoreBuilder::make_subgraph_store_and_pools( | ||
&self.logger, | ||
|
@@ -582,6 +609,29 @@ async fn main() { | |
sleep, | ||
) | ||
} | ||
Run { | ||
network_name, | ||
subgraph, | ||
stop_block, | ||
} => { | ||
let logger = ctx.logger.clone(); | ||
let config = ctx.config(); | ||
let registry = ctx.metrics_registry().clone(); | ||
let node_id = ctx.node_id().clone(); | ||
let store_builder = ctx.store_builder().await; | ||
|
||
commands::run::run( | ||
logger, | ||
store_builder, | ||
network_name, | ||
config, | ||
registry, | ||
node_id, | ||
subgraph, | ||
stop_block, | ||
) | ||
.await | ||
} | ||
Listen(cmd) => { | ||
use ListenCommand::*; | ||
match cmd { | ||
|
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.
It seems a little weird that we stop after getting the block but before processing it - that way, the last block we'll actually process (and have entities for) will be
stop_block - 1
. It's probably fine, but will need to be documented ingraphman run
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.
The instance manager stops after seeing (and before processing) the block HIGHER THAN the stop block:
if block_ptr.number > stop_block
So the behavior is to process up to stop_block INCLUSIVE, as you would expect (well, it Works on My Machine ™️)