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

Parameterize TLS configuration options #243

Merged
merged 4 commits into from
Dec 20, 2024
Merged

Parameterize TLS configuration options #243

merged 4 commits into from
Dec 20, 2024

Conversation

gmfrasca
Copy link
Member

Update Pipeline to allow TLS Cert verification when connecting to Teacher- and Judge- models

Description

  • Add _use_tls switch params for sdg, mt_bench, and final_eval phases
  • Source certs from teacher- and judge-server ConfigMaps (already used for other metadata)
  • Update eval phases to create and use httpx client instead of a custom-patched openai client
  • Update README

How Has This Been Tested?

  • Run pipeline with certs and _use_tls params enabled, verify successful connections to both teacher and judge models during SDG and Eval phases
  • Run pipeline with certs turned off (and models requiring them), verify phases fail due to incorrect/missing certs error

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@gmfrasca
Parameterize TLS Certs for SDG and Eval model connections
5be3f90 
- Add _use_tls switch params for sdg, mt_bench, and final_eval phases
- Update http client generation for eval steps to create httpx instead
  of patching the OpenAI class with a custom class

Signed-off-by: Giulio Frasca <gfrasca@redhat.com>
@gmfrasca
Update README with Judge and Teacher Models TLS details
5dfa7bd 
Signed-off-by: Giulio Frasca <gfrasca@redhat.com>
@gmfrasca gmfrasca force-pushed the pipeline-models-use-tls branch from 206a854 to 5dfa7bd Compare December 20, 2024 00:55
@gmfrasca
Dynamically detect and use certs for model connections
7690ced 
Signed-off-by: Giulio Frasca <gfrasca@redhat.com>
@gmfrasca gmfrasca force-pushed the pipeline-models-use-tls branch 2 times, most recently from 9e40ff4 to 16aebf5 Compare December 20, 2024 20:26
@gmfrasca
Fix ruff format/lint issues
7c41734 
Signed-off-by: Giulio Frasca <gfrasca@redhat.com>
@gmfrasca gmfrasca force-pushed the pipeline-models-use-tls branch from 16aebf5 to 7c41734 Compare December 20, 2024 20:27
MichaelClifford
MichaelClifford approved these changes Dec 20, 2024
View reviewed changes
Copy link
Collaborator

@MichaelClifford MichaelClifford left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @gmfrasca

@MichaelClifford MichaelClifford merged commit 723fb6c into opendatahub-io:main Dec 20, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaelClifford MichaelClifford MichaelClifford 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.
2 participants
@gmfrasca @MichaelClifford