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

Add network info in config #485

Closed
wants to merge 2 commits into from
Closed

Conversation

harisang
Copy link
Contributor

@harisang harisang commented Jan 9, 2025

This PR adds a variable named network in the top-level of the AccountingConfig.

Addresses comment: #475 (comment)

@harisang harisang requested a review from fhenneke January 9, 2025 07:49
Copy link
Collaborator

@fhenneke fhenneke left a comment

Choose a reason for hiding this comment

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

I would propose to use the approach of #483 instead of merging this PR.

It is also reasonable to add the network to the config. For the thing you want to achieve here, it seems you rather want to have a network name in the sync specific config object. Having a dictionary to map networks to network names should not happen in the main code. Such stuff needs to handled in the config.

@@ -54,8 +53,14 @@ async def sync_data_to_db( # pylint: disable=too-many-arguments
)
# we note that the block range computed above is meant to be interpreted as
# a closed interval
network_name_map = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems you you are trying to get to the dune name of networks. This can already be done by using config.dune_config.dune_blockchain. See #483, where the network is not passed and the dune config is used to get the network name.

@harisang
Copy link
Contributor Author

harisang commented Jan 9, 2025

Probably not needed, as explained here:
#485 (comment)

@harisang harisang closed this Jan 9, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Jan 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants