Skip to content

Dialplan idea#121

Open
ovv wants to merge 3 commits intomasterfrom
dialplan-idea
Open

Dialplan idea#121
ovv wants to merge 3 commits intomasterfrom
dialplan-idea

Conversation

@ovv
Copy link
Copy Markdown
Contributor

@ovv ovv commented May 8, 2018

This is a quick implementation of my idea about dialplan (third commit) by implementing a dialplan that handle authentication.

ovv added 3 commits May 7, 2018 14:59
Some request can come without a `To` headers tag (after a 401 for
example). This should probably be done only for specific
status code.
@vodik
Copy link
Copy Markdown
Contributor

vodik commented May 10, 2018

I think I like the look at this. Funny enough I was looking at doing the same thing right now.

I'll check it out and play with it locally and give you some feedback.

On a different note - how comfortable would you be with using python-attrs package? Its not an extra dep for me as I already mix this with aiohttp.

@vodik vodik mentioned this pull request May 10, 2018
Comment thread aiosip/dialplan.py
method, message, local_addr, remote_addr, protocol)


class AuthDialplan(BaseDialplan):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm okay taking this, but I suspect we don't actually need classes in the public API.

An async generator and closure should be sufficient to capture everything.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I like class because they can be easily subclassed. I'll have to try your proposition make my mind

@ovv
Copy link
Copy Markdown
Contributor Author

ovv commented May 10, 2018

I haven't used attrs much but I suspect it can be quite useful in aiosip. Feel free to pull it in.

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