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.

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

feat: add proxysettings for azureopenai and openai #415

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tanujd11
Copy link

@tanujd11 tanujd11 commented Apr 17, 2024

Closes #307

πŸ“‘ Description

This PR adds proxyEndpoint to be configured from the operator. The feature was provided by k8sgpt-ai/k8sgpt#987 in k8sgpt.

βœ… Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

β„Ή Additional Information

@tanujd11 tanujd11 requested review from a team as code owners April 17, 2024 10:11
Signed-off-by: tanujd11 <dwiveditanuj41@gmail.com>
@lawrencelo
Copy link

We are waiting for this proxy setting feature in next k8sgpt-operator release. Currently it does not take proxyEndpoint in k8sgpt CRD spec. Thanks in advance for moving this forward.

Copy link
Member

@arbreezy arbreezy left a comment

Choose a reason for hiding this comment

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

thank you @tanujd11 , a few comments:

  • Update the docs to include the new spec's attribute
  • I was checking the k8sgpt's PR that we merged recently, we missed to surface the proxyEndpoint as a cli argument , feel free to add it if you want
  • You have to update manually the helm's chart CRD template to include the
proxyEndpoint:
   type: string  

@tanujd11
Copy link
Author

  • You have to update manually the helm's chart CRD template to include the

@arbreezy I have already did the change in the file you mentioned. PTAL.

@arbreezy
Copy link
Member

  • You have to update manually the helm's chart CRD template to include the

@arbreezy I have already did the change in the file you mentioned. PTAL.

you are right,
the only pending update is to add the new spec attribute in the README

@arbreezy
Copy link
Member

@tanujd11 just bumping a message, changes lgtm but please update README's example to include the new spec attribute you are creating πŸ™

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Question]: Adding proxy settings in the k8sgpt-operator
3 participants