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

feat: get logs batching #658

Merged
merged 5 commits into from
May 21, 2024
Merged

feat: get logs batching #658

merged 5 commits into from
May 21, 2024

Conversation

platonfloria
Copy link
Collaborator

No description provided.

fastlane_bot/config/constants.py Show resolved Hide resolved
fastlane_bot/events/event_gatherer.py Outdated Show resolved Hide resolved
fastlane_bot/config/constants.py Show resolved Hide resolved
fastlane_bot/events/event_gatherer.py Outdated Show resolved Hide resolved
@barakman barakman self-requested a review May 21, 2024 09:08
fastlane_bot/events/event_gatherer.py Outdated Show resolved Hide resolved
run_blockchain_terraformer.py Outdated Show resolved Hide resolved
@barakman barakman self-requested a review May 21, 2024 10:04
fastlane_bot/events/event_gatherer.py Outdated Show resolved Hide resolved
@zavelevsky zavelevsky requested a review from barakman May 21, 2024 10:39
@@ -30,6 +32,7 @@ def __init__(
manager: The Manager object
w3: The connected AsyncWeb3 object.
"""
self._blockchain = blockchain
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can obtain this from self.cfg.network.NETWORK or something similar, rather than creating a duplicated attribute.

Copy link
Collaborator

@barakman barakman May 21, 2024

Choose a reason for hiding this comment

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

Or maybe self.cfg.NETWORK...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

EventGatherer has no access to the config object

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, then it is probably better to add the Config object rather than the blockchain ID, since it ensures that the same blockchain IDs are used across the code (i.e., it helps preventing mistakes, so one just needs to pass the same Config object which is used everywhere else in the system).

@barakman barakman self-requested a review May 21, 2024 11:20
@@ -30,6 +32,7 @@ def __init__(
manager: The Manager object
w3: The connected AsyncWeb3 object.
"""
self._blockchain = blockchain
Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, then it is probably better to add the Config object rather than the blockchain ID, since it ensures that the same blockchain IDs are used across the code (i.e., it helps preventing mistakes, so one just needs to pass the same Config object which is used everywhere else in the system).

@platonfloria platonfloria merged commit 62cd989 into sei-release-candidate May 21, 2024
1 check passed
@platonfloria platonfloria deleted the batch-get-logs branch May 21, 2024 12:04
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