Skip to content
This repository was archived by the owner on Jun 26, 2023. It is now read-only.

Conversation

@maschad
Copy link
Member

@maschad maschad commented Apr 21, 2023

Given that getDialQueue calls the internal connection manager's getDialQueue method, which would return the current pendingDials, it's expected this would include an array of PendingDials containing the promises of connections , this updates the interface to reflect that.

@maschad maschad requested a review from achingbrain April 21, 2023 19:30
@achingbrain
Copy link
Member

Could you add a bit of context here please? What problem does this solve?

@maschad maschad changed the title fix!:added promise to PendingDial interface fix: added promise to PendingDial interface Apr 22, 2023
@maschad
Copy link
Member Author

maschad commented Apr 22, 2023

Could you add a bit of context here please? What problem does this solve?

My apologies, I have updated the description

@achingbrain
Copy link
Member

Not including it in the interface was somewhat intentional to make it harder for callers to modify the internal state of the queue.

This API was meant to allow inspection of the queue only.

On this basis we might be better off excluding it from the list items?

If modification of the queue is desirable one thing that might be worth adding is a cancel method to remove a pending dial from the queue and reject all promises waiting on it?

@maschad
Copy link
Member Author

maschad commented Apr 22, 2023

If modification of the queue is desirable one thing that might be worth adding is a cancel method to remove a pending dial from the queue and reject all promises waiting on it?

I agree with this, although the connection manager is ultimately responsible for handling dials, this would give consumers the option should they not want to use the default connection manager.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants