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.

Sign up for GitHub

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

Jump to bottom

Standardize Endpoints and API Keys for AML Chat #37

Merged
merged 8 commits into from
Feb 9, 2024
Merged

Standardize Endpoints and API Keys for AML Chat #37

merged 8 commits into from
Feb 9, 2024

Conversation

rlundeen2
Copy link
Contributor

@rlundeen2 rlundeen2 commented Feb 8, 2024 •
edited
Loading

Description

Running the AML notebooks, they weren't working due to not configuring endpoints. This PR makes the arguments more consistent with how we want to handle default endpoints and keys.

  • checks/gets default values for AML endpoints. This makes it more consistent with AzureOpenAI
  • Adds a way to try to get required default values from environment variables if not provided as an argument

Tests

  • no new tests required
  • new tests added
  • existing tests adjusted

Documentation

  • no documentation changes needed
  • documentation added or edited
  • example notebook added or updated

@rlundeen2 rlundeen2 changed the title Users/rlundeen/2 7 aml default Standardize Endpoints and API Keys for AML Chat Feb 8, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 8, 2024 •
edited
Loading

Test Results

91 tests  +6   91 ✅ +6   9s ⏱️ +2s
 1 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 06e8097. ± Comparison against base commit bfeaab4.

♻️ This comment has been updated with latest results.

romanlutz
romanlutz reviewed Feb 8, 2024
View reviewed changes
.env_example Outdated Show resolved Hide resolved
pyrit/chat/azure_openai_chat.py Outdated Show resolved Hide resolved
pyrit/common/environment_variables.py Show resolved Hide resolved
@rlundeen2 rlundeen2 merged commit 86d6781 into main Feb 9, 2024
7 checks passed
@rlundeen2 rlundeen2 deleted the users/rlundeen/2_7_aml_default branch February 9, 2024 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romanlutz romanlutz romanlutz left review comments

@jbolor21 jbolor21 jbolor21 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rlundeen2 @romanlutz @jbolor21