Skip to content
This repository has been archived by the owner on Apr 15, 2024. It is now read-only.

feat: implement p2p querier #111

Merged
merged 4 commits into from
Feb 11, 2023
Merged

feat: implement p2p querier #111

merged 4 commits into from
Feb 11, 2023

Conversation

rach-id
Copy link
Member

@rach-id rach-id commented Feb 9, 2023

Overview

Closes #90

Contributes to: #108

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@rach-id rach-id added the p2p p2p network related label Feb 9, 2023
@rach-id rach-id requested a review from rahulghangas February 9, 2023 12:34
@rach-id rach-id requested a review from evan-forbes as a code owner February 9, 2023 12:34
@rach-id rach-id self-assigned this Feb 9, 2023
@rach-id
Copy link
Member Author

rach-id commented Feb 9, 2023

Depends on: #110

Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

mostly LGTM, had one blocking question though

Comment on lines +176 to +177
// TODO: make the timeout configurable
time.Sleep(10 * time.Second)
Copy link
Member

Choose a reason for hiding this comment

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

what is this for?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just to keep a delay between the different calls to querying 2/3. I wanna change it with a ticker, what do you think? every 10sec

Copy link
Member

@evan-forbes evan-forbes Feb 10, 2023

Choose a reason for hiding this comment

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

I see, a ticker could be more explicit yeah, but can certainly be done outside of this PR and not the highest priority as you've indicated

Copy link
Member Author

Choose a reason for hiding this comment

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

evan-forbes
evan-forbes previously approved these changes Feb 10, 2023
# Conflicts:
#	relayer/relayer.go
@rach-id
Copy link
Member Author

rach-id commented Feb 10, 2023

@evan-forbes Can you re-approve please? just resolved the conflicts

@rach-id rach-id merged commit f37279e into celestiaorg:main Feb 11, 2023
@rach-id rach-id deleted the p2p_querier branch February 11, 2023 07:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p2p p2p network related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement the P2P querier
2 participants