Opened 3 years ago

Closed 3 years ago

#20037 closed enhancement (fixed)

Add support for Bifroest's bridge descriptor tarballs

Reported by: karsten Owned by:
Priority: Medium Milestone:
Component: Metrics/CollecTor Version:
Severity: Normal Keywords:
Cc: iwakeh Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The current bridge authority Tonga will be shut down in a few days. The new bridge authority Bifroest is up and running for a few days now and will take over. See #19690 for details about the transition.

We'll have to handle Bifroest's bridge descriptor taballs, which we can't do right now. I'll post a branch in a second.

Child Tickets

Change History (6)

comment:1 Changed 3 years ago by karsten

Status: newneeds_review

Please review my branch task-20037.

comment:2 Changed 3 years ago by iwakeh

It would be great to have some tests for this addition. Is there time for that?
If not these should be added as soon as possible.
Maybe add some more inline comments with possible test cases now that will help adding tests later?

The tests will be very important when huge methods like in BridgeSnapshotReader (just the constructor has more than 200 lines) or in SanitizedBridgesWriter (almost 200 lines in one method) will undergo refactoring.

comment:3 Changed 3 years ago by karsten

We won't have time to add tests before deploying, because, erm, it's deployed as of 60 minutes ago. And I guess I should push what I'm running, in particular as it didn't break. Unless I shouldn't?!

But we can always add more tests going forward. How about we do that together with the other bridgedescs module tests that I wrote on some other ticket?

comment:4 in reply to:  3 Changed 3 years ago by iwakeh

Replying to karsten:

We won't have time to add tests before deploying, because, erm, it's deployed as of 60 minutes ago. And I guess I should push what I'm running, in particular as it didn't break. Unless I shouldn't?!

Well, if its running it should be committed, so we know what is running.
Not breaking doesn't necessarily mean 'working as expected' to me. (I hope it does here.)

But we can always add more tests going forward. How about we do that together with the other bridgedescs module tests that I wrote on some other ticket?

Yes, that would be very good and sort of the only type of documentation for that code.
Of course, the code itself tells something, but often it does not exactly what was expected by the author. These tests will also reduce maintenance time a lot in future.

comment:5 Changed 3 years ago by iwakeh

Status: needs_reviewmerge_ready

added reminder to #19755 for additional tests; this could be closed after merge then.

comment:6 Changed 3 years ago by karsten

Resolution: fixed
Status: merge_readyclosed

Merged, closing. Thanks!

Note: See TracTickets for help on using tickets.