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

Sync the value files with Components and Example #698

Merged
merged 2 commits into from
Jan 16, 2025

Conversation

yongfengdu
Copy link
Collaborator

Description

Sync the value files with GenAIExample and Components.
The copies in GenAIInfra is necessary now for CI tests, we'll have a yaml file to record copies on other repos, and have a script to check the difference.
This can be enhanced later if we come up with another better solution.

Issues

List the issue or RFC link this PR is working on. If there is no such link, please mark it as n/a.

Type of change

List the type of change like below. Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds new functionality)
  • Breaking change (fix or feature that would break existing design and interface)

Dependencies

List the newly introduced 3rd party dependency if exists.

Tests

Describe the tests that you ran to verify your changes.

@lianhao
Copy link
Collaborator

lianhao commented Jan 16, 2025

Could you please also remove the line ci-*values.yaml in the corresponding .helmignore files?

Rename custom values files for ci change, and keep consistent with Example and Components.
Remove values files link begin with ci.
Add a script and mapping file to track sync status of duplicated file between repos.
Add default minimal resource requests for all charts.

Signed-off-by: Dolpher Du <[email protected]>
@yongfengdu yongfengdu requested a review from chensuyue January 16, 2025 06:53
@yongfengdu
Copy link
Collaborator Author

8 failing checks:

agentqna:

First time enable the CI test, lacking of directory in CI server.
Have follow up changes and will fix the ci issue together.

chatqna x 2, faqgen x 2:
Have follow up changes to adapt the llm-uservice change.

lvm-serve, guardrails, mm-embedding:
Lacking of docker images in the ci server

We'll merge this PR first and make sure the above CI will succeed with follow up fixes.
Any changes in this PR will trigger ~60 helm e2e test, which is too heavy.

Copy link
Collaborator

@lianhao lianhao left a comment

Choose a reason for hiding this comment

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

Fast track to merge this first, will fix the errors in the upcoming PRs.

@lianhao lianhao merged commit d5cfeb4 into opea-project:main Jan 16, 2025
62 of 70 checks passed
@lianhao
Copy link
Collaborator

lianhao commented Jan 16, 2025

8 failing checks:

agentqna:

First time enable the CI test, lacking of directory in CI server. Have follow up changes and will fix the ci issue together.

chatqna x 2, faqgen x 2: Have follow up changes to adapt the llm-uservice change.

lvm-serve, guardrails, mm-embedding: Lacking of docker images in the ci server

We'll merge this PR first and make sure the above CI will succeed with follow up fixes. Any changes in this PR will trigger ~60 helm e2e test, which is too heavy.

PR #710 should resolve faqgen, guardrails CI failure.

Chatqna changes depends on all 3 PRs #710, #711, #712 to land in first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants