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

KAZOO-5262: Make number search timeout configurable #3078

Merged
merged 1 commit into from Jan 4, 2017
Merged

KAZOO-5262: Make number search timeout configurable #3078

merged 1 commit into from Jan 4, 2017

Conversation

onnet
Copy link
Contributor

@onnet onnet commented Jan 4, 2017

I'm testing Telnyx as a number provider atm.
Sometimes Telnyx processes its number lookup slightly longer than 5 seconds.
Because of 5000 number search hardcoded timeout, Kazoo provides empty result...

@@ -57,6 +57,8 @@

-define(POLLING_INTERVAL, 5000).

-define(NUMBER_SEARCH_TIMEOUT, kapps_config:get_integer(?KNM_CONFIG_CAT, <<"number_search_timeout">>, 5000)).
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you suffix this to signify milliseconds? ie "number_search_timeout_ms"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fenollp Done.

@fenollp
Copy link
Contributor

fenollp commented Jan 4, 2017

I have experienced the same issue. Thanks for this patch! :)

@onnet
Copy link
Contributor Author

onnet commented Jan 4, 2017

@fenollp Hi Pierre! There is another question raised with Telnyx module. Number lookup returns numbers with different activation charges and monthly fees costs. Since, at the moment, Kazoo charges numbers fees depending on configured service plans, there could be some impact from business point of view :)) Do you have any ideas about it already?

@onnet
Copy link
Contributor Author

onnet commented Jan 4, 2017

Look at the second line of the table:

telnyxnumbers

@fenollp
Copy link
Contributor

fenollp commented Jan 4, 2017

Hey I personally agree with your approach at #3081
Let's hear @lazedo 's opinion on this?

@fenollp
Copy link
Contributor

fenollp commented Jan 4, 2017

Merging this. Let's discuss over at #3081

@fenollp fenollp merged commit c8fde5b into 2600hz:master Jan 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants