Skip to content
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 Agent Configuration by File #396

Merged
merged 1 commit into from
Jun 16, 2022
Merged

Indexer Agent Configuration by File #396

merged 1 commit into from
Jun 16, 2022

Conversation

kaiwetlesen
Copy link

** RECREATING THIS PR** from PR #395 as the CI pipeline didn't like building off of a fork.

This pull requests implements file-based configuration for the indexer agent such that the configuration file supplements base defaults for a cluster. The idea here is that default values for mainnet, rinkeby, etc can be supplied as code in the repository, with more important parameters being injected via environment variables and CLI flags.

The order of precedence is as follows:

CLI flags
Environment variables
Configuration file parameters

F.eks. running indexer-agent with a configuration file containing ethereum: https://ethereum-node-0:8545 and setting INDEXER_AGENT_ETHEREUM to https://ethereum.hosted.service/accountid/, the effective setting would be https://ethereum.hosted.service/accountid/.

@kaiwetlesen
Copy link
Author

kaiwetlesen commented Apr 18, 2022

Hey @fordN , @chriswessels , and @hopeyen , I've merged in changes from main, which cleaned this PR right up. Ready for review.

Also, the format check for yarn.lock seems to fail, complaining that I'm adding a new package. Ideas?

@hopeyen
Copy link
Contributor

hopeyen commented Apr 19, 2022

Could you try removing yarn.lock and re yarn?

fordN
fordN previously requested changes Apr 19, 2022
packages/indexer-agent/src/commands/start.ts Outdated Show resolved Hide resolved
@fordN
Copy link
Contributor

fordN commented Apr 19, 2022

What do you think about submitting a similar PR for the indexer-service @kaiwetlesen?

@fordN
Copy link
Contributor

fordN commented Apr 19, 2022

Hey @fordN , @chriswessels , and @hopeyen , I've merged in changes from main, which cleaned this PR right up. Ready for review.

Also, the format check for yarn.lock seems to fail, complaining that I'm adding a new package. Ideas?

Can you clean up your commit history here so there is an easily followable path? There should only be one commit for this big of a change and no merge commits.

@kaiwetlesen kaiwetlesen requested a review from fordN June 9, 2022 19:16
@kaiwetlesen kaiwetlesen dismissed fordN’s stale review June 9, 2022 19:16

Incorporated changes

@fordN
Copy link
Contributor

fordN commented Jun 14, 2022

@kaiwetlesen this is still showing 14 commits and includes merge commits. Please rebase on main and squash your commits to a single commit, it's only a 15 line change...

@kaiwetlesen
Copy link
Author

kaiwetlesen commented Jun 14, 2022

this is still showing 14 commits and includes merge commits. Please rebase on main and squash your commits to a single commit, it's only a 15 line change...

@fordN done. Misunderstood what you'd asked for before.

Copy link
Contributor

@fordN fordN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@kaiwetlesen kaiwetlesen merged commit 8b04ce4 into main Jun 16, 2022
@kaiwetlesen kaiwetlesen deleted the kaiw/config-file branch June 16, 2022 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants