-
Notifications
You must be signed in to change notification settings - Fork 3
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
refactor: Remove async indexer_rules_engine
#387
Conversation
@@ -0,0 +1,3551 @@ | |||
{ |
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.
Added as indexer_rules_engine
tests depend on it
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.
LGTM!
@@ -204,6 +204,7 @@ mod tests { | |||
/// Parses env vars from .env, Run with | |||
/// cargo test s3::tests::list_delta_bucket -- mainnet from-latest; | |||
#[tokio::test] | |||
#[ignore] |
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.
Did you intend to leave all of these ignores around?
EDIT: Nevermind. I read your commit message again. Why did the tests not work anymore actually?
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.
They do, but they rely on external sources and AWS access keys so I didn't want to run them in CI. They're mostly run locally so in that case the #[ignore]
can be removed then.
We have two versions of
indexer_rules_engine
; sync and async. This logic does not need to beasync
so I've removed this version and refactored so only the sync version is used.Additionally, I've migrated the
async
tests across to use thesync
version, and have addedcargo test
to Github Actions so that they are always run. This required fixing theset_provisioning_flag
test, as well as ignoring the integration tests which rely on AWS access keys.