[sr-dev] [kamailio/kamailio] schema: allow null in active_watchers reason (#1345)

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[sr-dev] [kamailio/kamailio] schema: allow null in active_watchers reason (#1345)

Victor Seva-3

some code paths in presence module set the reason field to null causing problems updating the database.


You can view, comment on, or merge this pull request online at:

  https://github.com/kamailio/kamailio/pull/1345

Commit Summary

  • schema: allow null in active_watchers reason

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/kamailio/kamailio","title":"kamailio/kamailio","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/kamailio/kamailio"}},"updates":{"snippets":[{"icon":"DESCRIPTION","message":"schema: allow null in active_watchers reason (#1345)"}],"action":{"name":"View Pull Request","url":"https://github.com/kamailio/kamailio/pull/1345"}}}</script>
_______________________________________________
Kamailio (SER) - Development Mailing List
[hidden email]
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Reply | Threaded
Open this post in threaded view
|

Re: [kamailio/kamailio] schema: allow null in active_watchers reason (#1345)

Victor Seva-3

Many columns default to empty string because the code expect it and doesn't do additional checks to see if the db value is NULL. Have you checked the code for this case?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/kamailio/kamailio","title":"kamailio/kamailio","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/kamailio/kamailio"}},"updates":{"snippets":[{"icon":"PERSON","message":"@miconda in #1345: Many columns default to empty string because the code expect it and doesn't do additional checks to see if the db value is NULL. Have you checked the code for this case?"}],"action":{"name":"View Pull Request","url":"https://github.com/kamailio/kamailio/pull/1345#issuecomment-347816292"}}}</script>
_______________________________________________
Kamailio (SER) - Development Mailing List
[hidden email]
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Reply | Threaded
Open this post in threaded view
|

Re: [kamailio/kamailio] schema: allow null in active_watchers reason (#1345)

Victor Seva-3
In reply to this post by Victor Seva-3

@miconda on new dialog subscribes the value is null. we can either add a default or allow null


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/kamailio/kamailio","title":"kamailio/kamailio","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/kamailio/kamailio"}},"updates":{"snippets":[{"icon":"PERSON","message":"@lazedo in #1345: @miconda on new dialog subscribes the value is null. we can either add a `default` or allow null"}],"action":{"name":"View Pull Request","url":"https://github.com/kamailio/kamailio/pull/1345#issuecomment-347819035"}}}</script>
_______________________________________________
Kamailio (SER) - Development Mailing List
[hidden email]
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Reply | Threaded
Open this post in threaded view
|

Re: [kamailio/kamailio] schema: allow null in active_watchers reason (#1345)

Victor Seva-3
In reply to this post by Victor Seva-3

Have you tested with the updated schema and it works?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/kamailio/kamailio","title":"kamailio/kamailio","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/kamailio/kamailio"}},"updates":{"snippets":[{"icon":"PERSON","message":"@miconda in #1345: Have you tested with the updated schema and it works?"}],"action":{"name":"View Pull Request","url":"https://github.com/kamailio/kamailio/pull/1345#issuecomment-347885301"}}}</script>
_______________________________________________
Kamailio (SER) - Development Mailing List
[hidden email]
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Reply | Threaded
Open this post in threaded view
|

Re: [kamailio/kamailio] schema: allow null in active_watchers reason (#1345)

Victor Seva-3
In reply to this post by Victor Seva-3
Hi Daniel,

only with sqlite which uses kamailio db routines to generate the actual sql
statement.
i can try others or, maybe i'm missing something basic in sqlite.

On Wed, Nov 29, 2017 at 2:57 PM, Daniel-Constantin Mierla <
[hidden email]> wrote:

> Have you tested with the updated schema and it works?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/kamailio/kamailio/pull/1345#issuecomment-347885301>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAvDrKirdug78Mq7Wi19QMcVUIOQzoBOks5s7XDEgaJpZM4QuuDc>
> .
>


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/kamailio/kamailio","title":"kamailio/kamailio","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/kamailio/kamailio"}},"updates":{"snippets":[{"icon":"PERSON","message":"@lazedo in #1345: Hi Daniel,\n\nonly with sqlite which uses kamailio db routines to generate the actual sql\nstatement.\ni can try others or, maybe i'm missing something basic in sqlite.\n\nOn Wed, Nov 29, 2017 at 2:57 PM, Daniel-Constantin Mierla \u003c\nnotifications@github.com\u003e wrote:\n\n\u003e Have you tested with the updated schema and it works?\n\u003e\n\u003e —\n\u003e You are receiving this because you authored the thread.\n\u003e Reply to this email directly, view it on GitHub\n\u003e \u003chttps://github.com/kamailio/kamailio/pull/1345#issuecomment-347885301\u003e,\n\u003e or mute the thread\n\u003e \u003chttps://github.com/notifications/unsubscribe-auth/AAvDrKirdug78Mq7Wi19QMcVUIOQzoBOks5s7XDEgaJpZM4QuuDc\u003e\n\u003e .\n\u003e\n"}],"action":{"name":"View Pull Request","url":"https://github.com/kamailio/kamailio/pull/1345#issuecomment-347894458"}}}</script>
_______________________________________________
Kamailio (SER) - Development Mailing List
[hidden email]
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Reply | Threaded
Open this post in threaded view
|

Re: [kamailio/kamailio] schema: allow null in active_watchers reason (#1345)

Victor Seva-3
In reply to this post by Victor Seva-3

I checked mysql and it doesn't allow NULL -- if some parts of the code sets it to NULL, then it should have been an error as well. Can you point to such code?

I am fine to change it to allow NULL, just to be sure there is no side effect.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/kamailio/kamailio","title":"kamailio/kamailio","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/kamailio/kamailio"}},"updates":{"snippets":[{"icon":"PERSON","message":"@miconda in #1345: I checked mysql and it doesn't allow NULL -- if some parts of the code sets it to NULL, then it should have been an error as well. Can you point to such code?\r\n\r\nI am fine to change it to allow NULL, just to be sure there is no side effect."}],"action":{"name":"View Pull Request","url":"https://github.com/kamailio/kamailio/pull/1345#issuecomment-349687412"}}}</script>
_______________________________________________
Kamailio (SER) - Development Mailing List
[hidden email]
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Reply | Threaded
Open this post in threaded view
|

Re: [kamailio/kamailio] schema: allow null in active_watchers reason (#1345)

Victor Seva-3
In reply to this post by Victor Seva-3

@miconda hi Daniel,
follow the code path of handle_subscribe in presence/subscriber.c that leads to update_subscription

  • new subscription
  • pres_notifier_processes = 0
  • dialog subscription => event->req_auth == 0

this code doesn't change the value its null by default, the code path with req_auth != 0, ~line 1202 does set it explicitly to null before calling the subscription handler.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/kamailio/kamailio","title":"kamailio/kamailio","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/kamailio/kamailio"}},"updates":{"snippets":[{"icon":"PERSON","message":"@lazedo in #1345: @miconda hi Daniel,\r\nfollow the code path of handle_subscribe in presence/subscriber.c that leads to `update_subscription`\r\n- new subscription\r\n- pres_notifier_processes = 0\r\n- dialog subscription =\u003e event-\u003ereq_auth == 0\r\n\r\nthis code doesn't change the value its null by default, the code path with req_auth != 0, ~line 1202 does set it explicitly to null before calling the subscription handler.\r\n"}],"action":{"name":"View Pull Request","url":"https://github.com/kamailio/kamailio/pull/1345#issuecomment-349723840"}}}</script>
_______________________________________________
Kamailio (SER) - Development Mailing List
[hidden email]
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Reply | Threaded
Open this post in threaded view
|

Re: [kamailio/kamailio] schema: allow null in active_watchers reason (#1345)

Victor Seva-3
In reply to this post by Victor Seva-3

Merged #1345.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/kamailio/kamailio","title":"kamailio/kamailio","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/kamailio/kamailio"}},"updates":{"snippets":[{"icon":"DESCRIPTION","message":"Merged #1345."}],"action":{"name":"View Pull Request","url":"https://github.com/kamailio/kamailio/pull/1345#event-1379944226"}}}</script>
_______________________________________________
Kamailio (SER) - Development Mailing List
[hidden email]
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev