Fix the inference generator shell variable substitution

opendatahub-io / ai-edge

ODH integration with AI at the Edge usecases

Apache License 2.0

8 stars 17 forks source link

Fix the inference generator shell variable substitution #253

Closed devguyio closed 4 months ago

devguyio commented 4 months ago

Description

Fixing the inference generator shell scripts. The SRVC_URL might have a wrong variable substitution syntax.

How Has This Been Tested?

Was not tested. It's pretty generic knowledge about shell variable substitution.

Merge criteria:

[ ] The commits are squashed in a cohesive manner and have meaningful messages.
[ ] Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
[ ] The developer has manually tested the changes and verified that the changes work

openshift-ci[bot] commented 4 months ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from devguyio. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/opendatahub-io/ai-edge/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

adelton commented 4 months ago

Nack.

I don't think this is a problem or needed. Per https://kubernetes.io/docs/tasks/inject-data-application/define-environment-variable-container/ and https://kubernetes.io/docs/tasks/inject-data-application/define-interdependent-environment-variables/ the envs get expanded with the $(VARNAME) syntax.

devguyio commented 4 months ago

Nack.

I don't think this is a problem or needed. Per https://kubernetes.io/docs/tasks/inject-data-application/define-environment-variable-container/ and https://kubernetes.io/docs/tasks/inject-data-application/define-interdependent-environment-variables/ the envs get expanded with the $(VARNAME) syntax.

Yes indeed! I completely forgot about that. Thanks.

/close

openshift-ci[bot] commented 4 months ago

@devguyio: Closed this PR.

In response to [this](https://github.com/opendatahub-io/ai-edge/pull/253#issuecomment-2069787028): >> Nack. >> >> I don't think this is a problem or needed. Per https://kubernetes.io/docs/tasks/inject-data-application/define-environment-variable-container/ and https://kubernetes.io/docs/tasks/inject-data-application/define-interdependent-environment-variables/ the `env`s get expanded with the `$(VARNAME)` syntax. > >Yes indeed! I completely forgot about that. Thanks. > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.