Skip to content

Conversation

@lianhao
Copy link
Contributor

@lianhao lianhao commented Apr 23, 2025

Copy link
Collaborator

@lkk12014402 lkk12014402 left a comment

Choose a reason for hiding this comment

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

LGTM. This will benefit network limited users

Copy link

@yipandeng yipandeng left a comment

Choose a reason for hiding this comment

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

LGTM.

@yinghu5 yinghu5 requested a review from Copilot April 23, 2025 06:11
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces an RFC to support air-gapped environments for OPEA microservices by outlining the online data types and proposing verification methods.

  • Adds a new RFC document outlining air-gap support for microservices.
  • Details different online data types and proposed handling methods.

Copy link
Collaborator

@mkbhanda mkbhanda left a comment

Choose a reason for hiding this comment

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

looks good. Awaiting AMD comments.
PS, fixed grammar only because copilot was engaged, wanted to see what it missed!

@lianhao
Copy link
Contributor Author

lianhao commented Apr 23, 2025

Fixed all grammar typo

@mkbhanda
Copy link
Collaborator

Have we captured that the tool we build should be executable by the application owner/deployer to ensure OPEA is not violating any re-distribution constraints/license agreements. The tool like our other scripts should take arguments or environment variable, a union of all the environment variables across the application pipeline like HuggingFace token, access token to private container and data repositories etc.

@eero-t
Copy link
Contributor

eero-t commented Apr 23, 2025

Have we captured that the tool we build should be executable by the application owner/deployer to ensure OPEA is not violating any re-distribution constraints/license agreements. The tool like our other scripts should take arguments or environment variable, a union of all the environment variables across the application pipeline like HuggingFace token, access token to private container and data repositories etc.

@mkbhanda Models can be downloaded from HF only if owner of given HF token has accepted their licenses.

In general licenses also require that license is included when data is re-distributed, but license info seems to be included only with a subset of models used by OPEA (maybe other models are not downloaded from HF, and/or are public domain?):

$ ls */snapshots/*/LICENSE*
models--Qwen--Qwen2.5-Coder-7B-Instruct/snapshots/0eb6b1ed2d0c4306bc637d09ecef51e59d3dfe05/LICENSE
models--meta-llama--Llama-3.2-3B-Instruct/snapshots/0cb88a4f764b7a12671c53f0838cd831a0843b95/LICENSE.txt
models--meta-llama--Meta-Llama-3-8B-Instruct/snapshots/5f0b02c75b57c5855da9ae460ce51323ea669d8a/LICENSE

@lianhao lianhao force-pushed the air-gapped branch 2 times, most recently from 9988aab to 67a016c Compare April 24, 2025 05:26
Copy link
Contributor

@eero-t eero-t left a comment

Choose a reason for hiding this comment

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

Approved, but consider the remaining 4 suggestions.

@lianhao lianhao force-pushed the air-gapped branch 2 times, most recently from 67a016c to e268bc0 Compare April 25, 2025 01:26
@yinghu5
Copy link
Collaborator

yinghu5 commented May 7, 2025

Hi @lianhao, there is a conversation is not resolved. please check. [remind]

Signed-off-by: Lianhao Lu <lianhao.lu@intel.com>
Co-authored-by: Eero Tamminen <eero.t.tamminen@intel.com>
@lianhao
Copy link
Contributor Author

lianhao commented May 7, 2025

Hi @lianhao, there is a conversation is not resolved. please check. [remind]

Should be all resolved now.

@yinghu5 yinghu5 merged commit 37eb5a7 into opea-project:main May 9, 2025
4 checks passed
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.

7 participants