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

[WIP] add test for Raw kserve auth #91

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mwaykole
Copy link
Member

No description provided.

Signed-off-by: Milind Waykole <mwaykole@mwaykole-thinkpadp1gen4i.bengluru.csb>
Copy link

The following are automatically added/executed:

Available user actions:

  • To mark a PR as WIP, add /wip in a comment. To remove it from the PR comment /wip cancel to the PR.
  • To block merging of a PR, add /hold in a comment. To un-block merging of PR comment /hold cancel.
  • To mark a PR as approved, add /lgtm in a comment. To remove, add /lgtm cancel.
    lgtm label removed on each new commit push.
  • To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
    verified label removed on each new commit push.
Supported labels

{'/wip', '/verified', '/hold', '/lgtm'}

Signed-off-by: Milind Waykole <mwaykole@mwaykole-thinkpadp1gen4i.bengluru.csb>
Comment on lines +191 to +194
return run_command(
command=shlex.split(f"oc create token -n {model_service_account.namespace} {model_service_account.name}")
)[1].strip()

Copy link
Collaborator

Choose a reason for hiding this comment

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

please create a function and re-use in all places

"model_namespace, s3_models_storage_uri",
[
pytest.param(
{"name": "kserve-token-authentication"},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
{"name": "kserve-token-authentication"},
{"name": "kserve-raw-token-authentication"},

],
indirect=True,
)
class TestKserveTokenAuthenticationRaw:
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's split to rest and grpc; then test cross-domain with one protocol. this way we can re-use existing fixtures and not add new ones.

@@ -90,7 +91,7 @@ def create_isvc(
if deployment_mode == KServeDeploymentType.SERVERLESS:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if deployment_mode == KServeDeploymentType.SERVERLESS:
if deployment_mode in (KServeDeploymentType.SERVERLESS, KServeDeploymentType.RAW_DEPLOYMENT):

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

Successfully merging this pull request may close these issues.

2 participants