search

opea-project / GenAIComps

GenAI components at micro-service level; GenAI service composer to create mega-service
Apache License 2.0
81 stars 142 forks source link

[Bug] Some sub-microservices aren't following docker image naming convention detailed in docs repo. #962

Open jjmaturino opened 3 days ago

jjmaturino commented 3 days ago

Priority

Undecided

OS type

Ubuntu

Hardware type

Xeon-GNR

Installation method

Deploy method

Running nodes

Single Node

What's the version?

N/A

Description

When looking at the docs and viewing the naming conventions for docker images of micro services, I came across multiple different micro-services images that aren't adhering to the standard.

One example is in the llms/text-generation micro-service folder.

Current Image Name: image: opea/llm-tgi:latest Current Standard: opea/[microservice type]-[microservice sub type]-[library name]-[vendor]-[hardware]:latest Proposed Changes:` image: opea/llm-text-generation-tgi:latest

Another potential solution would be to make the subtype section optional/ remove it alltogether.

Reproduce steps

Look through repo.

Raw log

No response

Attachments

No response

jjmaturino commented 3 days ago

@lvliang-intel tagging you, since I see you're very active on this repo.

jjmaturino commented 3 days ago

Okay, I might of misspoken... It seems like they're correctly defined in this file, but some of the README.md are outdated?

jjmaturino commented 3 days ago

Might be able to close this issue. Would like someone else to verify; considering I am new to this repo.

Thanks!

feng-intel commented 3 days ago

Thanks for your suggestion. What's the matter does current naming bring to you?

-- some of the README.md are outdated Could you give examples?

jjmaturino commented 3 days ago

https://github.com/opea-project/GenAIComps/blob/0772494f624eb315d617934bbf4dc3f244475080/comps/llms/text-generation/tgi/docker_compose_llm.yaml#L24

Okay just found one where there is a discrepancy between what is build and pushed to the docker repo and what is in the codebase. This one is probably a bug

jjmaturino commented 3 days ago

https://github.com/opea-project/GenAIComps/blob/main/comps/llms/text-generation/tgi/README.md

All over a bunch of readme's theres a discrepancy between the naming convention and the tag used while building the docker image in the examples.

jjmaturino commented 3 days ago

If I have a base llm docker image that implements opea standard, it doesn't make much sense to me to have a llm-docsum, llm-chat, llm-faq docker image, considering that each python script is using the same llm docker image, just modifying how it interacts with that llm through potentially different prompts.

It would make sense to me for a vendor to have one docker image for the microservice, and then the sub microservices implement the base image as an interface for whatever the functionality of the microservice provides.

so for example docsum would use the base image of llm-[vendor] as would all the other vendor specific submicroservices.

jjmaturino commented 3 days ago

There was also a PR a while back that renamed a bunch of things... and from what I can tell doesn't adhere to the standard.

For some context, I was looking at the PR[https://github.com/opea-project/GenAIComps/commit/f19cf083d1b8cba7acadc761851c1b73686519c3#diff-f078c91c4ef0062ff6aa2c054b994acd1965444573000196b99ad50b7ad74256] that merged the text-generation for langchain.

It seems like the subfolders that use the langchain docker image all utilize vllm-gaudi, similar for the tgi subfolder.

jjmaturino commented 2 days ago

This is an unrelated question, but how do you all handle development for this repo.

Do you develop using docker with hot-reloads?

Or do you use poetry? Pip with virtualenv?

Would love to hear your about your DX experience.

eero-t commented 2 days ago

I think answer is "all of above and more".

For example, this is my development flow for stuff in this repo (I'm contributor, not member):

=> Killing the pod means service is automatically restarted with the changes

Note: If that cluster had only single node, or pod container could always access the repo where my changes are, above could be simplified a bit. In first case changes could be just mounted directly from local fs to container, in latter case, taken directly from the repo.

[1] Charts for components here are moving from GenAIInfra to this repo, and there are (hopefully) going to be also other changes, to reduce duplication in the repos, before v1.2 release. Which should help in making changes requested in this ticket...