Skip to content

MBS-10621: Custom endpoints without azure options#118

Open
Snl-FGH wants to merge 16 commits intomainfrom
MBS-10621-Custom_endpoints_without_azure_options
Open

MBS-10621: Custom endpoints without azure options#118
Snl-FGH wants to merge 16 commits intomainfrom
MBS-10621-Custom_endpoints_without_azure_options

Conversation

@Snl-FGH
Copy link
Copy Markdown
Contributor

@Snl-FGH Snl-FGH commented Mar 2, 2026

No description provided.

@PhMemmel PhMemmel changed the title Mbs 10621 custom endpoints without azure options MBS-10621: Custom endpoints without azure options Mar 2, 2026
@Snl-FGH Snl-FGH force-pushed the MBS-10621-Custom_endpoints_without_azure_options branch from c61068b to f4d0d0b Compare March 18, 2026 08:36
Copy link
Copy Markdown
Member

@PhMemmel PhMemmel left a comment

Choose a reason for hiding this comment

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

I have not yet completed the initial review, but sending this over to you so far for a first round.

@Snl-FGH Snl-FGH force-pushed the MBS-10621-Custom_endpoints_without_azure_options branch from 400e458 to 4571569 Compare March 27, 2026 21:45
. $deploymentid . '/chat/completions?api-version=' . $apiversion;
// We have an empty model because the model is preconfigured if we're using azure.
// So we overwrite the default "preconfigured" value by a better model name.
$enabled = aitool_option_azure::extract_azure_data_to_store($data);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

well, it's a little break in the data structure maybe. There were multiple attributes, you now reduce the return value of the function to just one return value.

On the other hand add_azure_options_to_form_data is staying with returning a whole object.

Not sure if we need to change this again. It just feels a little bit inconsistent. But on the other hand probably won't matter :)

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.

You're right, it's a bit asymmetric. The two functions just serve different purposes, so their return types ended up different. Happy to change it if you feel strongly about it though :)

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