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: add info logger to display connection status and chain ID #2268

Closed
wants to merge 2 commits into from

Conversation

Ninjagod1251
Copy link
Contributor

@Ninjagod1251 Ninjagod1251 commented Sep 5, 2024

What I did

  • added logging to the start method to differentiate between local test chains and other networks

How I did it

Modify the start method in the provider to include additional logging.

after confirming the provider is connected, the method checks the chain_id. If the chain_id is 1337, it logs a message indicating a connection to a local test chain.

How to verify it

  1. Run the Provider: Start the provider process using the modified start method.
  2. Check Logs: Verify that the correct logging messages are displayed:
    • If the provider is connected to a local test chain with chain_id=1337, ensure the log message reflects this.
    • For other networks, check that the log message indicates the connected network and its chain ID.

Checklist

  • All changes are completed
  • New test cases have been added
  • Documentation has been updated

@Ninjagod1251
Copy link
Contributor Author

Im writing the test atm

@Ninjagod1251 Ninjagod1251 self-assigned this Sep 5, 2024
Copy link
Member

@antazoey antazoey left a comment

Choose a reason for hiding this comment

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

the logic is off!

my suggestion: add a logger line to ape_test/provider.py instead

@@ -1075,7 +1089,12 @@ def start(self, timeout: int = 20):
with RPCTimeoutError(self, seconds=timeout) as _timeout:
while True:
if self.is_connected:
break
chain_id = self.chain_id # Ensure this is how chain_id is accessed
if chain_id == 1337:
Copy link
Member

Choose a reason for hiding this comment

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

The chain ID is configurable and does not mean it is an Ethereum tester node. Any node can use this port!

Copy link

github-actions bot commented Oct 6, 2024

This pull request is considered stale because it has been open 30 days with no activity. Remove stale label, add a comment, or make a new commit, otherwise this PR will be closed in 5 days.

@github-actions github-actions bot added the stale No activity for 30 days label Oct 6, 2024
Copy link

This PR was closed because it has been inactive for 35 days.

@github-actions github-actions bot added the inactive no recent activity, closed label Oct 12, 2024
@github-actions github-actions bot closed this Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inactive no recent activity, closed stale No activity for 30 days
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants