Re: [asterisk-dev] [Code Review] 4285: Bug fixes for ARI Originate/Continue with label support (Continuation of /r/4101)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4285/ --- (Updated Jan. 7, 2015, 12:53 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 430337 Bugs: ASTERISK-24412 https://issues.asterisk.org/jira/browse/ASTERISK-24412 Repository: Asterisk Description --- See /r/4101 for the initial review uploaded by Nir Simionivich to understand the need for the ability to originate/continue to a labeled dialplan priority. When writing the tests in /r/4284 for continuation/origination, I ran into some problems, and so I have updated the original patch to fix these issues. Here are the modifications to the original patch: * In continuation code, we get a channel snapshot from the stasis_app_control instead of getting an actual channel instance. This is because pbx_findlabel_extension doesn't actually use the channel for anything, so the channel isn't necessary. * In both continuation and origination code, do not pass the input context to pbx_findlabel_extension. If no context is specified, this causes a crash. Instead, determine the intended context beforehand and pass that context into pbx_findlabel_extension. * This is not a fault of the previous patch, but there were code paths that resulted in some unexpected behavior in continuation. Since continuation states that context, extension, priority, and label are all optional, but didn't really do anything to make sense of what should happen when one or more are omitted, the code has been updated to make sure that a sane default is used when one or more of these are omitted. * I have updated the apiVersion in rest-api/resources.json to 1.7.0 since this change introduces backwards-compatible new functionality. QUESTION: Is this version bump required in other .json files as well? * I have updated CHANGES to indicate the new functionality Diffs - /branches/13/rest-api/resources.json 430178 /branches/13/rest-api/api-docs/channels.json 430178 /branches/13/res/res_ari_channels.c 430178 /branches/13/res/ari/resource_channels.c 430178 /branches/13/res/ari/resource_channels.h 430178 /branches/13/CHANGES 430178 Diff: https://reviewboard.asterisk.org/r/4285/diff/ Testing --- See /r/4284 for tests. Thanks, Mark Michelson -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4285: Bug fixes for ARI Originate/Continue with label support (Continuation of /r/4101)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4285/#review14098 --- Ship it! Ship It! - Joshua Colp On Jan. 6, 2015, 10:21 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4285/ --- (Updated Jan. 6, 2015, 10:21 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24412 https://issues.asterisk.org/jira/browse/ASTERISK-24412 Repository: Asterisk Description --- See /r/4101 for the initial review uploaded by Nir Simionivich to understand the need for the ability to originate/continue to a labeled dialplan priority. When writing the tests in /r/4284 for continuation/origination, I ran into some problems, and so I have updated the original patch to fix these issues. Here are the modifications to the original patch: * In continuation code, we get a channel snapshot from the stasis_app_control instead of getting an actual channel instance. This is because pbx_findlabel_extension doesn't actually use the channel for anything, so the channel isn't necessary. * In both continuation and origination code, do not pass the input context to pbx_findlabel_extension. If no context is specified, this causes a crash. Instead, determine the intended context beforehand and pass that context into pbx_findlabel_extension. * This is not a fault of the previous patch, but there were code paths that resulted in some unexpected behavior in continuation. Since continuation states that context, extension, priority, and label are all optional, but didn't really do anything to make sense of what should happen when one or more are omitted, the code has been updated to make sure that a sane default is used when one or more of these are omitted. * I have updated the apiVersion in rest-api/resources.json to 1.7.0 since this change introduces backwards-compatible new functionality. QUESTION: Is this version bump required in other .json files as well? * I have updated CHANGES to indicate the new functionality Diffs - /branches/13/rest-api/resources.json 430178 /branches/13/rest-api/api-docs/channels.json 430178 /branches/13/res/res_ari_channels.c 430178 /branches/13/res/ari/resource_channels.c 430178 /branches/13/res/ari/resource_channels.h 430178 /branches/13/CHANGES 430178 Diff: https://reviewboard.asterisk.org/r/4285/diff/ Testing --- See /r/4284 for tests. Thanks, Mark Michelson -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4285: Bug fixes for ARI Originate/Continue with label support (Continuation of /r/4101)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4285/ --- (Updated Jan. 6, 2015, 10:21 p.m.) Review request for Asterisk Developers. Changes --- Added same error message to origination as well. Bugs: ASTERISK-24412 https://issues.asterisk.org/jira/browse/ASTERISK-24412 Repository: Asterisk Description --- See /r/4101 for the initial review uploaded by Nir Simionivich to understand the need for the ability to originate/continue to a labeled dialplan priority. When writing the tests in /r/4284 for continuation/origination, I ran into some problems, and so I have updated the original patch to fix these issues. Here are the modifications to the original patch: * In continuation code, we get a channel snapshot from the stasis_app_control instead of getting an actual channel instance. This is because pbx_findlabel_extension doesn't actually use the channel for anything, so the channel isn't necessary. * In both continuation and origination code, do not pass the input context to pbx_findlabel_extension. If no context is specified, this causes a crash. Instead, determine the intended context beforehand and pass that context into pbx_findlabel_extension. * This is not a fault of the previous patch, but there were code paths that resulted in some unexpected behavior in continuation. Since continuation states that context, extension, priority, and label are all optional, but didn't really do anything to make sense of what should happen when one or more are omitted, the code has been updated to make sure that a sane default is used when one or more of these are omitted. * I have updated the apiVersion in rest-api/resources.json to 1.7.0 since this change introduces backwards-compatible new functionality. QUESTION: Is this version bump required in other .json files as well? * I have updated CHANGES to indicate the new functionality Diffs (updated) - /branches/13/rest-api/resources.json 430178 /branches/13/rest-api/api-docs/channels.json 430178 /branches/13/res/res_ari_channels.c 430178 /branches/13/res/ari/resource_channels.c 430178 /branches/13/res/ari/resource_channels.h 430178 /branches/13/CHANGES 430178 Diff: https://reviewboard.asterisk.org/r/4285/diff/ Testing --- See /r/4284 for tests. Thanks, Mark Michelson -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4285: Bug fixes for ARI Originate/Continue with label support (Continuation of /r/4101)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4285/ --- (Updated Jan. 6, 2015, 10:20 p.m.) Review request for Asterisk Developers. Changes --- Changed the error message Josh pointed out to simply point out that the entered priority label is invalid. Bugs: ASTERISK-24412 https://issues.asterisk.org/jira/browse/ASTERISK-24412 Repository: Asterisk Description --- See /r/4101 for the initial review uploaded by Nir Simionivich to understand the need for the ability to originate/continue to a labeled dialplan priority. When writing the tests in /r/4284 for continuation/origination, I ran into some problems, and so I have updated the original patch to fix these issues. Here are the modifications to the original patch: * In continuation code, we get a channel snapshot from the stasis_app_control instead of getting an actual channel instance. This is because pbx_findlabel_extension doesn't actually use the channel for anything, so the channel isn't necessary. * In both continuation and origination code, do not pass the input context to pbx_findlabel_extension. If no context is specified, this causes a crash. Instead, determine the intended context beforehand and pass that context into pbx_findlabel_extension. * This is not a fault of the previous patch, but there were code paths that resulted in some unexpected behavior in continuation. Since continuation states that context, extension, priority, and label are all optional, but didn't really do anything to make sense of what should happen when one or more are omitted, the code has been updated to make sure that a sane default is used when one or more of these are omitted. * I have updated the apiVersion in rest-api/resources.json to 1.7.0 since this change introduces backwards-compatible new functionality. QUESTION: Is this version bump required in other .json files as well? * I have updated CHANGES to indicate the new functionality Diffs (updated) - /branches/13/rest-api/resources.json 430178 /branches/13/rest-api/api-docs/channels.json 430178 /branches/13/res/res_ari_channels.c 430178 /branches/13/res/ari/resource_channels.c 430178 /branches/13/res/ari/resource_channels.h 430178 /branches/13/CHANGES 430178 Diff: https://reviewboard.asterisk.org/r/4285/diff/ Testing --- See /r/4284 for tests. Thanks, Mark Michelson -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4285: Bug fixes for ARI Originate/Continue with label support (Continuation of /r/4101)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4285/#review14029 --- /branches/13/res/ari/resource_channels.c https://reviewboard.asterisk.org/r/4285/#comment24540 I don't think this is the correct message to put here. They didn't request priority 0, it just so happens that what they provided yielded us to having a priority of 0. I could see something being confused if they saw this. - Joshua Colp On Dec. 19, 2014, 5:15 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4285/ --- (Updated Dec. 19, 2014, 5:15 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24412 https://issues.asterisk.org/jira/browse/ASTERISK-24412 Repository: Asterisk Description --- See /r/4101 for the initial review uploaded by Nir Simionivich to understand the need for the ability to originate/continue to a labeled dialplan priority. When writing the tests in /r/4284 for continuation/origination, I ran into some problems, and so I have updated the original patch to fix these issues. Here are the modifications to the original patch: * In continuation code, we get a channel snapshot from the stasis_app_control instead of getting an actual channel instance. This is because pbx_findlabel_extension doesn't actually use the channel for anything, so the channel isn't necessary. * In both continuation and origination code, do not pass the input context to pbx_findlabel_extension. If no context is specified, this causes a crash. Instead, determine the intended context beforehand and pass that context into pbx_findlabel_extension. * This is not a fault of the previous patch, but there were code paths that resulted in some unexpected behavior in continuation. Since continuation states that context, extension, priority, and label are all optional, but didn't really do anything to make sense of what should happen when one or more are omitted, the code has been updated to make sure that a sane default is used when one or more of these are omitted. * I have updated the apiVersion in rest-api/resources.json to 1.7.0 since this change introduces backwards-compatible new functionality. QUESTION: Is this version bump required in other .json files as well? * I have updated CHANGES to indicate the new functionality Diffs - /branches/13/rest-api/resources.json 429698 /branches/13/rest-api/api-docs/channels.json 429698 /branches/13/res/res_ari_channels.c 429698 /branches/13/res/ari/resource_channels.c 429698 /branches/13/res/ari/resource_channels.h 429698 /branches/13/CHANGES 429698 Diff: https://reviewboard.asterisk.org/r/4285/diff/ Testing --- See /r/4284 for tests. Thanks, Mark Michelson -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
