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 geant lg agent #29

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

add geant lg agent #29

wants to merge 6 commits into from

Conversation

Laur04
Copy link

@Laur04 Laur04 commented Nov 5, 2022

I'm opening a new PR since I switched branches, but the old one is here for comment reference. I fixed the bugs...looks like it works now.

  • A new branch is a good idea; I just got lazy since I was working in my fork. The branch has been changed and I rebased it against main. Do you want me to target another branch with my PR as well?
  • Oops, didn't realize I'd committed those. Deleted.
  • Dependency added.
  • I think I've got docker-compose correct, but it got slightly complicated when rebasing, so if you could verify, that'd be great. I just commented out the CERN container for now.

A couple other comments:

  • The default update time is set as 300sec, but it takes a while to actually get through all of the GEANT routers. Is there any merit in extending that time?
  • Not every query returns a peer ("learned-from") for some reason. The code handles it without failing, but I'm not sure how that might affect what's built on top of this.
  • Many (~10?) of the routers time out each update() cycle, but it's not the same 10 every cycle. It might be beneficial to limit the time given for each request to minimize the delays from these timeouts.

@Laur04 Laur04 force-pushed the geant-lg branch 2 times, most recently from e3f0741 to aecbcc4 Compare November 6, 2022 00:44
@fno2010
Copy link
Member

fno2010 commented Nov 9, 2022

It seems that the batch query cannot work well. The response will be 500 once any of the selectedRouters fails.

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.

2 participants