opea-project / GenAIExamples

Generative AI Examples is a collection of GenAI examples such as ChatQnA, Copilot, which illustrate the pipeline capabilities of the Open Platform for Enterprise AI (OPEA) project.
https://opea.dev
Apache License 2.0
282 stars 195 forks source link

[Bug]Docs ask user to set container_id environment variable #971

Closed mkbhanda closed 4 weeks ago

mkbhanda commented 1 month ago

Priority

P4-Low

OS type

Ubuntu

Hardware type

CPU-other (Please let us know in description)

Installation method

Deploy method

Running nodes

Single Node

What's the version?

https://github.com/opea-project/GenAIExamples/commit/c930bea172d535ca24c51a4419f08547190747e7

Description

1) For completeness of instructions, particularly when we need to build the docker images, for those specified in GenAIComps, add an instruction to git clone the GenAIComps repo. Also the Docker file refers to the $PYTHON_PATH, need our users to set it.

2) We can improve the documentation in https://github.com/opea-project/GenAIExamples/tree/5dae7137932b0f397e2bfe18bfd2cd94da0dd495/ChatQnA/docker_compose/intel/cpu/xeon by asking the user to set the container_id environment variable. docker logs ${CONTAINER_ID} | grep Connected

Better still if we name the LLM service more generically, without mentioning the model serving framework even, such as "llm_service" the command can be even more generic requiring no environment variable setting.

docker logs llm_service | grep Connected

3) While building the docker images a) for "retriever" and Mega service 1 warning found (use docker --debug to expand):

Follow readme guide.

Raw log

devcloud@ubuntu:~$ docker logs ${CONTAINER_ID} | grep Connected
"docker logs" requires exactly 1 argument.
See 'docker logs --help'.

Usage:  docker logs [OPTIONS] CONTAINER

Fetch the logs of a container
louie-tsai commented 1 month ago

We indeed could use container_name defined in the compose.yaml file and it works. image

The only potential issue is having two docker instance with same name, but it should not happen if user uses docker_compose. if no concern to use container_name defined in compose.yaml, we could change readme accordingly. What is your take?

also agree on first request, and will work on that too.

louie-tsai commented 1 month ago

update ChatQnA accordingly in below PR https://github.com/opea-project/GenAIExamples/pull/1018

louie-tsai commented 1 month ago

Let us know any remining issue in the latest main branch. if no, we will close it for now.