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: Update ollama-curated.yaml #488

Merged
merged 2 commits into from
Jul 12, 2024
Merged

Conversation

HavenDV
Copy link
Contributor

@HavenDV HavenDV commented Jul 12, 2024

No description provided.

@HavenDV
Copy link
Contributor Author

HavenDV commented Jul 12, 2024

Regarding AnyOf - I did the same as it is done in the OpenAI OpenAPI specification, this will allow users to be able to access new values ​​from the SDK without the need to update, and also makes it possible to do more correct logging of what is happening because there are values ​​like this (for PullModel)
image

I can provide it in separate PR

@davidmigloz davidmigloz added t:enhancement New feature or request p:ollama_dart ollama_dart package. labels Jul 12, 2024
@davidmigloz davidmigloz self-requested a review July 12, 2024 21:42
@davidmigloz
Copy link
Owner

davidmigloz commented Jul 12, 2024

Thanks one more time for keeping the spec up-to-date!

And sorry, I totally forgot to reply to your comment about the anyOf in the previous PR.

I totally agree with using anyOf in the case you point out in the OpenAI spec. model is an "input" field and changes frequently. We do use it in our OpenAI client as well.

But for output fields that change very rarely, like the done_reason, I don't think it's worth the verbosity.

Currently, you can just do:

if(response.doneReason == DoneReason.stop) {...}

If you want to do the same supporting anyOf, it would be:

if(response.doneReason.mapOrNull(enumeration: (v) => v) == DoneReason.stop) {...}

Which is very verbose and hard to read (at least in Dart).

Currently, if Ollama adds a new doneReason, the client will just return null. But most likely, your code won't be handling the new value anyway. When you want to start handling it, you can just update the client to the version that supports it.

For the case of PushModelStatus, I think the best thing would be to change the type from enum to just string. As its values are not standardized. So it doesn't make much sense to use an enum there.

@davidmigloz davidmigloz added this to the v0.8.0 milestone Jul 12, 2024
@davidmigloz davidmigloz merged commit a110ecb into davidmigloz:main Jul 12, 2024
1 check passed
@davidmigloz
Copy link
Owner

(I've changed the type of the push model status field to String in #489)

KennethKnudsen97 pushed a commit to KennethKnudsen97/langchain_dart that referenced this pull request Oct 1, 2024
Co-authored-by: David Miguel <me@davidmiguel.com>
KennethKnudsen97 pushed a commit to KennethKnudsen97/langchain_dart that referenced this pull request Oct 1, 2024
Co-authored-by: David Miguel <me@davidmiguel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p:ollama_dart ollama_dart package. t:enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants