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

Merged
merged 3 commits into from
Sep 7, 2024

Conversation

tanujd11
Copy link
Contributor

@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
@tanujd11 tanujd11 force-pushed the feat/proxySettings branch from 2ea60a3 to 9e78c71 Compare April 17, 2024 10:13
@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
Contributor 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 πŸ™

@tanujd11 tanujd11 requested a review from arbreezy September 5, 2024 06:58
@tanujd11
Copy link
Contributor Author

tanujd11 commented Sep 5, 2024

@arbreezy, Could you take a look? I have updated the Readme.

@tanujd11
Copy link
Contributor Author

tanujd11 commented Sep 5, 2024

Can this be merged now?

@arbreezy arbreezy merged commit 7e7769c into k8sgpt-ai:main Sep 7, 2024
7 checks passed
@tanujd11 tanujd11 deleted the feat/proxySettings branch September 7, 2024 18:48
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