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

ns: executor supports environment variables #12

Merged
merged 2 commits into from
Feb 19, 2024
Merged

ns: executor supports environment variables #12

merged 2 commits into from
Feb 19, 2024

Conversation

derekbit
Copy link
Member

Which issue(s) this PR fixes:

Issue longhorn/longhorn#7672

What this PR does / why we need it:

Special notes for your reviewer:

Additional documentation or context

@derekbit derekbit self-assigned this Jan 25, 2024
c3y1huang
c3y1huang reviewed Feb 1, 2024
View reviewed changes
types/file.go Outdated Show resolved Hide resolved
c3y1huang
c3y1huang requested changes Feb 1, 2024
View reviewed changes
Copy link
Contributor

@c3y1huang c3y1huang left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The related unit tests are currently broken, please adopt the changes.

@c3y1huang c3y1huang mentioned this pull request Feb 1, 2024
Merged
@derekbit
Copy link
Member Author

derekbit commented Feb 1, 2024

The related unit tests are currently broken, please adopt the changes.

Fixed. Thank you.
There are many broken tests that are nothing to do with the PR. Will you fix them?

@c3y1huang
Copy link
Contributor

There are many broken tests that are nothing to do with the PR. Will you fix them?

Yes, thank you for noticing, I will have the fix in another pull request.

This was referenced Feb 2, 2024
Merged
Merged
@derekbit derekbit enabled auto-merge (rebase) February 2, 2024 02:31
@derekbit
Copy link
Member Author

derekbit commented Feb 2, 2024

@c3y1huang
Do you know why the merge is blocked?
The commit in the PR is already signed.

The base branch requires all commits to be signed.

c3y1huang
c3y1huang previously approved these changes Feb 2, 2024
View reviewed changes
@c3y1huang c3y1huang enabled auto-merge (rebase) February 2, 2024 02:36
@c3y1huang
Copy link
Contributor

c3y1huang commented Feb 2, 2024 •
edited
Loading

@c3y1huang Do you know why the merge is blocked? The commit in the PR is already signed.

The base branch requires all commits to be signed.

No idea, having the same blocking in #15. cc @innobead

@derekbit derekbit mentioned this pull request Feb 7, 2024
Merged
innobead
innobead requested changes Feb 19, 2024
View reviewed changes
ns/executor.go Outdated Show resolved Hide resolved
ns/executor.go Show resolved Hide resolved
@derekbit
ns: executor supports environment variables
049ef34 
Longhorn 7672

Signed-off-by: Derek Su <[email protected]>
@derekbit
test: add TestExecuteWithEnvs
c592afb 
Test namespaced execution with environment variables

Longhorn 7672

Signed-off-by: Derek Su <[email protected]>
@innobead innobead merged commit 081e7dc into longhorn:main Feb 19, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@innobead innobead innobead approved these changes

@c3y1huang c3y1huang Awaiting requested review from c3y1huang

Assignees

@derekbit derekbit

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@derekbit @c3y1huang @innobead