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

0001: Service Config #2

Merged
merged 10 commits into from
Dec 27, 2023
Merged

0001: Service Config #2

merged 10 commits into from
Dec 27, 2023

Conversation

roma-glushko
Copy link
Member

No description provided.

@roma-glushko roma-glushko added the enhancement New feature or request label Dec 16, 2023
@roma-glushko roma-glushko self-assigned this Dec 16, 2023
@roma-glushko roma-glushko marked this pull request as draft December 16, 2023 18:45
proposals/0001-gep.md Show resolved Hide resolved

### The Config Source

Glide config is represented as a YAML file (R1, R2, R3).
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. Should be YAML.

proposals/0001-gep.md Outdated Show resolved Hide resolved
- via Glide CLI params & args
- via environment variables
- via a separate config file
- via a combination of CLI, environment variables, and/or config file
Copy link
Contributor

Choose a reason for hiding this comment

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

How is a config file accessed by the providers?

Copy link
Member Author

Choose a reason for hiding this comment

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

Config is going to be a singleton inited and loaded in the Gateway class and then passed itself or specific config like HTTP Server Config to the corresponding structs that needs it. Something like that. I did not mention that detail here as the contract is more important to decide on and discuss then to focus on how we get there under the hood.

Copy link
Contributor

@mkrueger12 mkrueger12 left a comment

Choose a reason for hiding this comment

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

General comments

proposals/0001-gep.md Outdated Show resolved Hide resolved
Copy link
Contributor

@mkrueger12 mkrueger12 left a comment

Choose a reason for hiding this comment

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

updated config comment

…e most minimalistic config possible. Elaborated the reqs. Added rejected alternatives & future work
@roma-glushko roma-glushko marked this pull request as ready for review December 24, 2023 13:11
cert_path:
# other configs

routes:
Copy link
Member Author

Choose a reason for hiding this comment

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

@mkrueger12 our conversation around embeddings API in GEP0002 and ideas to support TTS & STT models have opened my eyes on the previously proposed strategy which is to have one uniform list of pools that support all API Glide providers.

I think that if you consider these two type of APIs, it's clear that the idea is not super viable. There is no reason to impose LLM lifecycle on something like STT/TTS. They are just too different. And we should take care of them considering their specific.

Language, embeddings, transcribers, synthesizers are just too different to treat them the same way and we might end up being in trouble trying to go that route.

So the new idea is to have type-specific model pools (e.g. language, embeddings, transcribers, synthesizers). This should aid separating logic needed (fallbacking, load balancing, unified request/response schemas, etc). for one type of models from logic that is appropriate in another.

The current config examples have reflected this approach.

Feel free to take a look and let know how does that feel 🙌

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep this makes complete sense. Lets go that route

This was referenced Dec 24, 2023
@roma-glushko roma-glushko linked an issue Dec 27, 2023 that may be closed by this pull request
@roma-glushko roma-glushko merged commit d4d7f89 into main Dec 27, 2023
@roma-glushko roma-glushko deleted the 0001-service-config branch December 27, 2023 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0001: Service Config
2 participants