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.

Sign up for GitHub

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

Jump to bottom

remove examples gateway. #979

Merged
merged 21 commits into from
Dec 13, 2024
Merged

remove examples gateway. #979

merged 21 commits into from
Dec 13, 2024

Conversation

lkk12014402
Copy link
Collaborator

Description

remove examples gateway after this pr opea-project/GenAIExamples#992 merged

ftian1
ftian1 reviewed Dec 6, 2024
View reviewed changes
comps/cores/mega/gateway.py Outdated Show resolved Hide resolved
@lkk12014402 lkk12014402 marked this pull request as draft December 9, 2024 14:48
@dmsuehir
Copy link
Contributor

dmsuehir commented Dec 9, 2024

@lkk12014402 Will the python gateway tests (such as https://github.com/opea-project/GenAIComps/blob/main/tests/cores/mega/test_multimodalqna_gateway.py) be moved to GenAIExamples?

@lkk12014402 lkk12014402 marked this pull request as ready for review December 10, 2024 07:01
@lkk12014402
Copy link
Collaborator Author

lkk12014402 commented Dec 10, 2024 •
edited
Loading

@lkk12014402 Will the python gateway tests (such as https://github.com/opea-project/GenAIComps/blob/main/tests/cores/mega/test_multimodalqna_gateway.py) be moved to GenAIExamples?

hi, for the multimodalqna test, there is a ut https://github.com/opea-project/GenAIExamples/tree/main/MultimodalQnA/tests in the GenAIExamples

ftian1
ftian1 reviewed Dec 10, 2024
View reviewed changes
comps/cores/mega/utils.py Outdated Show resolved Hide resolved
@Spycsh Spycsh self-requested a review December 10, 2024 07:12
@dmsuehir
Copy link
Contributor

@lkk12014402 Will the python gateway tests (such as https://github.com/opea-project/GenAIComps/blob/main/tests/cores/mega/test_multimodalqna_gateway.py) be moved to GenAIExamples?

hi, for the multimodalqna test, there is a ut https://github.com/opea-project/GenAIExamples/tree/main/MultimodalQnA/tests in the GenAIExamples

These are .sh script tests in GenAIExamples, but the ones in test_multimodalqna_gateway.py are python unit tests and it is stubbing out the other microservices in order to isolate testing the gateway, which I find pretty useful. I didn't see other python tests in GenAIExamples, so I wasn't sure if moving the .py test file to https://github.com/opea-project/GenAIExamples/tree/main/MultimodalQnA/tests would automatically work or if there would be more work associated with getting GHA to run those python test.

@joshuayao joshuayao linked an issue Dec 11, 2024 that may be closed by this pull request
[Feature] Move the functionalities of gateway to GenAIExamples #1026
Closed
@codecov Codecov
Copy link

codecov bot commented Dec 11, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 98.80952% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
comps/cores/mega/http_service.py 94.73% 1 Missing ⚠️
Files with missing lines Coverage Δ
comps/cores/mega/micro_service.py 80.72% <100.00%> (-2.33%) ⬇️
comps/cores/mega/utils.py 42.32% <100.00%> (+19.82%) ⬆️
comps/cores/mega/http_service.py 82.22% <94.73%> (+4.44%) ⬆️

... and 3 files with indirect coverage changes

@lkk12014402
Copy link
Collaborator Author

@lkk12014402 Will the python gateway tests (such as https://github.com/opea-project/GenAIComps/blob/main/tests/cores/mega/test_multimodalqna_gateway.py) be moved to GenAIExamples?

hi, for the multimodalqna test, there is a ut https://github.com/opea-project/GenAIExamples/tree/main/MultimodalQnA/tests in the GenAIExamples

These are .sh script tests in GenAIExamples, but the ones in test_multimodalqna_gateway.py are python unit tests and it is stubbing out the other microservices in order to isolate testing the gateway, which I find pretty useful. I didn't see other python tests in GenAIExamples, so I wasn't sure if moving the .py test file to https://github.com/opea-project/GenAIExamples/tree/main/MultimodalQnA/tests would automatically work or if there would be more work associated with getting GHA to run those python test.

ok, let's move the .py test of multimodalqna in GenAIComps to GenAIExamples

@lkk12014402 lkk12014402 force-pushed the remove_example_gateway branch 2 times, most recently from 47f6228 to d35617e Compare December 12, 2024 15:25
@lvliang-intel lvliang-intel merged commit f5efaf1 into main Dec 13, 2024
18 checks passed
@lvliang-intel lvliang-intel deleted the remove_example_gateway branch December 13, 2024 01:31
@lkk12014402 lkk12014402 mentioned this pull request Dec 13, 2024
Merged
@artem-astafev artem-astafev mentioned this pull request Dec 13, 2024
Merged
jjmaturino pushed a commit to jjmaturino/GenAIComps that referenced this pull request Dec 16, 2024
@lkk12014402 @pre-commit-ci
@chensuyue @jjmaturino
remove examples gateway. (opea-project#979)
da99090 
* remove examples gateway.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* remove gateway.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* refine service code.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update http_service.py

* remove gateway ut.

* remove gateway ut.

* fix conflict service name.

* Update http_service.py

* add handle message ut.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* remove `multiprocessing.Process` start server code.

* fix ut.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* remove multiprocessing and enhance ut for coverage.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: chen, suyue <[email protected]>
@chensuyue chensuyue mentioned this pull request Dec 18, 2024
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lvliang-intel lvliang-intel lvliang-intel approved these changes

@Spycsh Spycsh Spycsh approved these changes

@ftian1 ftian1 ftian1 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature] Move the functionalities of gateway to GenAIExamples
6 participants
@lkk12014402 @dmsuehir @ftian1 @Spycsh @lvliang-intel @chensuyue