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: Added Validation for Open AI API key #2412

Conversation

Daniyal-Qureshi
Copy link
Contributor

This PR aims to enhance the reliability and accuracy of the OpenAI key by validating the provided API Key.

Checklist

  • PR contains only changes related; no stray files, etc.
  • README updated (where applicable)
  • Tests written (where applicable)
  • References provided in PR (where applicable)

@Daniyal-Qureshi Daniyal-Qureshi force-pushed the feature/validate-open-AI-key branch from 7eaa745 to c1b39d4 Compare June 2, 2024 12:58
@Daniyal-Qureshi Daniyal-Qureshi changed the title Feat: Added Validation for Open AI key Feat: Added Validation for Open AI API key Jun 2, 2024
@rubiin
Copy link
Member

rubiin commented Jun 2, 2024

Not sure if we want to add a platform specific validator. We try to keep all the validator as generic as possible.
Lets hear from other maintainers in the mean time

@rubiin rubiin requested review from profnandaa, WikiRik and ezkemboi June 2, 2024 13:16
@WikiRik
Copy link
Member

WikiRik commented Jun 2, 2024

I'm against merging this in the current form. A possibility would be to have 'isAPIKey(string, provider)` where OpenAI would be one of the providers.
But even with that I think it would be better to use RegEx directly or a call towards OpenAI to validate the key. OpenAI could change the format of keys on a regular basis and we cannot give any guarantees with validator updates to accompany that.

@Daniyal-Qureshi
Copy link
Contributor Author

Daniyal-Qureshi commented Jun 2, 2024

@WikiRik Thank you for the feedback, I have not come across any method that open AI provides to validate the API key, I think we need to use the regex here. Also, we can make the method flexible by passing the provider, such as OpenAI, Gemini, Claude etc.

@rubiin
Copy link
Member

rubiin commented Jun 2, 2024

I'm against merging this in the current form. A possibility would be to have 'isAPIKey(string, provider)` where OpenAI would be one of the providers. But even with that I think it would be better to use RegEx directly or a call towards OpenAI to validate the key. OpenAI could change the format of keys on a regular basis and we cannot give any guarantees with validator updates to accompany that.

isAPIKey would be a better option . But keeping up with a vendor specific key sure looks like a hassle

@profnandaa
Copy link
Member

I agree, no need for a platform specific validator. I wouldn't be comfortable too with something like isAPIKey as we have often leaned towards validators against standards. I doubt there's a standard out there for an API Key?

@rubiin
Copy link
Member

rubiin commented Jun 3, 2024

I agree, no need for a platform specific validator. I wouldn't be comfortable too with something like isAPIKey as we have often leaned towards validators against standards. I doubt there's a standard out there for an API Key?

not really. everything that exists out there are vendor determined which are likely to change frequently or not at all

@profnandaa
Copy link
Member

Thanks for your suggestion @Daniyal-Qureshi ; please feel free to pick any of our other issues including cleaning up some old PRs that just needed minor updates. See #2410 Welcome to the project!

@profnandaa profnandaa closed this Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants