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

Fix query parameters of externalMedia #133

Closed

Conversation

hwiorn
Copy link

@hwiorn hwiorn commented Jan 5, 2022

Refer #118

Previous ExternalMedia implementation uses whole ExternalMediaOptions as a request body.
But asterisk only uses Variables field as the request body. So Asterisk 18(LTS) responds as 400 Bad Request which demand mandatory query parameters.
This patch uses ExternalMediaOptions as query parameters except a Variables which is a JSON format.

This fix is just only for the externalMedia API.
Go ARI uses similar approach in channel API(e.g. originate) like the externalMedia API that doesn't use query parameters.


This change is Reviewable

@@ -264,8 +264,8 @@ type ExternalMediaOptions struct {
// Direction specifies the directionality of the audio stream. Options include 'both'. This parameter is optional and if not specified, 'both' will be used.
Direction string `json:"direction"`

// Variables defines the set of channel variables which should be bound to this channel upon creation. This parameter is optional.
Variables map[string]string `json:"variables"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any need to remove Variables from ExternalMediaOptions struct and pass them as argument ?
Also data field was introduced with Asterisk 18, there is no such field in Asterisk 16 and its EOL is 2023-10-09.

I propose to ignore data field for now leaving struct as it is and only fix handling of ExternalMediaOptions for query.

@malygin
Copy link

malygin commented Apr 27, 2022

hey guys! It is unknown when the PR will be merged?

@Ulexus Ulexus deleted the branch CyCoreSystems:master September 5, 2023 14:08
@Ulexus Ulexus closed this Sep 5, 2023
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.

4 participants