Skip to content

Conversation

@mlazaric
Copy link
Contributor

@mlazaric mlazaric commented Jul 6, 2025

AZURE_OPENAI_API_KEY was previously an optional parameter and since only required parameters are visible in the UI, it was not possible to configure it. Let's make it required with an empty string as the default value. See also #2582.

Example from the UI
Screenshot 2025-07-06 at 17 10 53

AZURE_OPENAI_API_KEY was previously an optional parameter and since only required parameters are visible in the UI, it was not possible to configure it. Let's make it required with an empty string as the default value.
@angiejones
Copy link
Collaborator

I think this may confuse users who do not realize it's optional. Perhaps you can add "optional" in the label?

@michaelneale michaelneale self-assigned this Jul 8, 2025
Copy link
Collaborator

@michaelneale michaelneale left a comment

Choose a reason for hiding this comment

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

I can't manually validate it with azure but this seems to make sense

@michaelneale michaelneale added waiting bug p1 Priority 1 - High (supports roadmap) labels Jul 8, 2025
@DOsinga DOsinga removed the bug label Jul 8, 2025
@mlazaric
Copy link
Contributor Author

mlazaric commented Jul 8, 2025

I think this may confuse users who do not realize it's optional. Perhaps you can add "optional" in the label?

Good idea! I agree that it is a bit confusing, although I'm not sure what would be the best way to clarify it, so open to suggestions. I think we could rename AZURE_OPENAI_API_KEY to OPTIONAL_AZURE_OPENAI_API_KEY to be more explicit about it, but then it would not use the existing AZURE_OPENAI_API_KEY environment variable if it was set. Alternatively, we could make the default string be something like unused or optional. Curious what people prefer?

@michaelneale michaelneale added p3 Priority 3 - Low and removed p1 Priority 1 - High (supports roadmap) labels Jul 14, 2025
@michaelneale
Copy link
Collaborator

so if It has a value which says it is optional? (I am confused as to the aim now - ideally we wouldn't make an optional thing not optional?)

Copy link
Collaborator

@michaelneale michaelneale left a comment

Choose a reason for hiding this comment

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

need to clarify how it is actually still optional

Copy link
Collaborator

@michaelneale michaelneale left a comment

Choose a reason for hiding this comment

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

I think this is better than it was so am ok with it, and is isolated.

@michaelneale michaelneale merged commit c704319 into block:main Jul 14, 2025
10 of 11 checks passed
atarantino pushed a commit to atarantino/goose that referenced this pull request Jul 14, 2025
lifeizhou-ap added a commit that referenced this pull request Jul 15, 2025
* main:
  fix: convert invalid recipe variable name to raw content (#3420)
  center goose mobile screenshot (#3418)
  docs: model context limit overrides (#3377)
  docs: Subagents (#3402)
  fix: avoid pass encoded empty string to goose run --recipe (#3361)
  ux: alphabetize extensions (#3416)
  fix: message concatenation in server session management (#3412)
  refactor: streamline memory directory management (#3345)
  feat: Add AZURE_OPENAI_API_KEY as a visible config parameter (#3265)
  feat: stream LLM responses (#2677)
  fix checkout for non mac builds (#3408)
  Docs: Voice dictation in Goose Desktop (#3376)
  docs: cli theme persistence (#3398)
  docs: goose mobile (#3403)
zanesq added a commit that referenced this pull request Jul 15, 2025
* 'main' of github.com:block/goose:
  fix: Set include_usage=true for OpenAI streaming (#3441)
  feat: `recipe list` (#2814) (#2815)
  docs: update github mcp config (#3433)
  feat: Implement streaming for OpenAI (#3413)
  fix: improve extension startup error messages with command details (#2694)
  [feat]: improve file search tools to add globsearch / grep tools (#3368)
  docs: typo in guide description (#3429)
  fix: use safe_truncate to truncate charactor (#3263) (#3264)
  fix: convert invalid recipe variable name to raw content (#3420)
  center goose mobile screenshot (#3418)
  docs: model context limit overrides (#3377)
  docs: Subagents (#3402)
  fix: avoid pass encoded empty string to goose run --recipe (#3361)
  ux: alphabetize extensions (#3416)
  fix: message concatenation in server session management (#3412)
  refactor: streamline memory directory management (#3345)
  feat: Add AZURE_OPENAI_API_KEY as a visible config parameter (#3265)
  feat: stream LLM responses (#2677)

# Conflicts:
#	crates/goose/src/session/storage.rs
#	ui/desktop/src/components/ChatView.tsx
#	ui/desktop/src/components/settings/extensions/subcomponents/ExtensionList.tsx
s-soroosh pushed a commit to s-soroosh/goose that referenced this pull request Jul 18, 2025
kwsantiago pushed a commit to kwsantiago/goose that referenced this pull request Jul 19, 2025
cbruyndoncx pushed a commit to cbruyndoncx/goose that referenced this pull request Jul 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p3 Priority 3 - Low waiting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants