-
Notifications
You must be signed in to change notification settings - Fork 184
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
Changes to simulator usage #135
Conversation
…ex and use model_config for the latest version of the simulator
"metadata": {}, | ||
"outputs": [], | ||
"source": [ | ||
"os.environ[\"AZURE_OPENAI_API_KEY\"] = \"<your-api-key>\"\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move this section up to the Parameters cell and then reference the environment variables throughout instead of setting those values explicitly in the Parameters cell and then having this section below?
Then, users can know to set all of their environment variables and then run the notebook from there without any other configuration in later steps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slister1001 will address this in her PR - #138
I'm abandoning this PR once she has all the components from here included in hers
{ | ||
"content": "Here is my follow-up question.", | ||
"user": "user" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be ?
"role": "user"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Posting on #138
LGTM, just a minor comment. |
Description
Checklist