Skip to content

Fix unused variable, missing defaults, and silent misconfiguration in hotel booking sample#512

Merged
leestott merged 3 commits intomainfrom
copilot/fix-comments-from-review-thread
Apr 12, 2026
Merged

Fix unused variable, missing defaults, and silent misconfiguration in hotel booking sample#512
leestott merged 3 commits intomainfrom
copilot/fix-comments-from-review-thread

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 12, 2026

openai_api_key was read from the environment but never used, causing silent misconfiguration. GITHUB_ENDPOINT had no fallback (passing None to the client), and the model ID was hardcoded rather than environment-driven.

Changes

  • Fail fast on missing auth: removed unused openai_api_key; added explicit ValueError when GITHUB_TOKEN is unset
  • GITHUB_ENDPOINT default: falls back to https://models.inference.ai.azure.com instead of None
  • GITHUB_MODEL_ID env var: model ID is now read from the environment with "gpt-4o" as default
  • Remove duplicate env lookup: reuse the already-assigned github_token local variable in OpenAIChatClient
  • Comment accuracy: updated # GitHub Models or Azure OpenAI# GitHub Models
# Before
openai_api_key = os.getenv("OPENAI_API_KEY")          # unused
github_token = os.getenv("GITHUB_TOKEN")
chat_client = OpenAIChatClient(
    base_url=os.environ.get("GITHUB_ENDPOINT"),        # can be None
    api_key=os.environ.get("GITHUB_TOKEN"),            # duplicate lookup
    model_id="gpt-4o",                                 # hardcoded
)

# After
github_token = os.getenv("GITHUB_TOKEN")
if not github_token:
    raise ValueError("GITHUB_TOKEN environment variable is not set. Please set it to use GitHub Models.")
chat_client = OpenAIChatClient(
    base_url=os.environ.get("GITHUB_ENDPOINT", "https://models.inference.ai.azure.com"),
    api_key=github_token,
    model_id=os.environ.get("GITHUB_MODEL_ID", "gpt-4o"),
)

@github-actions
Copy link
Copy Markdown
Contributor

👋 Thanks for contributing @copilot! We will review the pull request and get back to you soon.

…alidation, use GITHUB_ENDPOINT default and GITHUB_MODEL_ID env var

Agent-Logs-Url: https://github.com/microsoft/ai-agents-for-beginners/sessions/e7564051-ab1f-4355-85a7-6dd6ecda986b

Co-authored-by: leestott <2511341+leestott@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix code based on review comments Fix unused variable, missing defaults, and silent misconfiguration in hotel booking sample Apr 12, 2026
Copilot AI requested a review from leestott April 12, 2026 07:49
@leestott
Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

@leestott leestott marked this pull request as ready for review April 12, 2026 07:54
Copilot AI review requested due to automatic review settings April 12, 2026 07:54
…le.py

Co-authored-by: leestott <2511341+leestott@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 12, 2026

@copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in commit 6a36d0e. The conflict was in hotel_booking_workflow_sample.py where origin/main had merged a MiniMax provider addition (PR #474). The resolved version uses origin/main's multi-provider implementation, which also includes all the original review fixes (GITHUB_ENDPOINT default, GITHUB_MODEL_ID env var, proper provider selection).

@leestott leestott merged commit ab885dc into main Apr 12, 2026
4 checks passed
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants