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

allow to pass context values to override environment variables #1439

Merged
merged 8 commits into from Jun 4, 2023
Merged

allow to pass context values to override environment variables #1439

merged 8 commits into from Jun 4, 2023

Conversation

atoulme
Copy link
Contributor

@atoulme atoulme commented Mar 26, 2023

Fixes #1438

@atoulme atoulme force-pushed the override_env branch 4 times, most recently from 712bc7b to 738801a Compare March 26, 2023 07:02
@dloucasfx dloucasfx mentioned this pull request Mar 27, 2023
Closed
@dloucasfx dloucasfx mentioned this pull request Mar 27, 2023
Closed
@dloucasfx
Copy link

@shirou when you get a chance, can you please review this PR ?
Thanks

@shirou
Copy link
Owner

shirou commented Apr 23, 2023

Sorry to late response. I finally had time to review it. As a premise, I agree with the policy about the method of putting values into the context.Context.

So, I have a more implementation-oriented comment. common.Env will become a map[string]string, which means that any value can be put in it, and the user does not know what value can be put in it. This amplifies the bad points of context.

Therefore, how about to restrict the types in the common package as follows. (If you have a better name idea, please change as you like)

type EnvKeyType string
const (
    HostProcEnvKey EnvKeyType = "HOST_PROC"
    HostSysEnvKey EnvKeyType = "HOST_SYS"
    HostEtcEnvKey EnvKeyType = "HOST_ETC"
      (snip)
)
type EnvMap map[EnvKeyType]string

GetEnvWithContext retrieve side may became a bit more complicated, but I think this method is clearer and more extensible. More to the point, in the future, I would also like to add options other than HOST_* here.

How do you think?

@atoulme
Copy link
Contributor Author

atoulme commented May 26, 2023

I'm on it.

@atoulme
Copy link
Contributor Author

atoulme commented May 26, 2023

Ready for another review @shirou , thanks!

shirou
shirou reviewed May 30, 2023
View reviewed changes
internal/common/common_linux.go Outdated Show resolved Hide resolved
internal/common/common_test.go Outdated Show resolved Hide resolved
internal/common/common_test.go Outdated Show resolved Hide resolved
@atoulme
Copy link
Contributor Author

atoulme commented Jun 3, 2023

@shirou code review feedback addressed, thanks!

shirou
shirou approved these changes Jun 4, 2023
View reviewed changes
Copy link
Owner

@shirou shirou left a comment

Choose a reason for hiding this comment

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

I would like to merge this as I think this is probably the best way to go.
Thank you very much for your patience with my review.

I believe this change will give gopsutil great potential for extensibility in the future. I would like to thank you again!

@shirou shirou merged commit 607d540 into shirou:master Jun 4, 2023
19 checks passed
shirou added a commit that referenced this pull request Jun 4, 2023
@shirou
add(README): add passing context value document
341d501 
which is introduced by #1439.
@shirou shirou mentioned this pull request Jun 4, 2023
Merged
@atoulme atoulme deleted the override_env branch June 28, 2023 20:51
@mx-psi mx-psi mentioned this pull request Jun 30, 2023
Merged
@shirou shirou mentioned this pull request Aug 13, 2023
Merged
chenjiandongx added a commit to TencentBlueKing/gopsutil that referenced this pull request Aug 29, 2023
@chenjiandongx @cuishuang
@shirou @dependabot @powersj @atoulme @gabibguti @elfranne @Lomanic @kishen-v @chouquette @ozanh @gdvalle
Sync: 同步上游 master 分支 (#6)
4a2f9f1 
* fix some comments

Signed-off-by: cui fliter <imcusg@gmail.com>

* chore(deps): bump github.com/stretchr/testify from 1.8.2 to 1.8.3

Bumps [github.com/stretchr/testify](https://github.com/stretchr/testify) from 1.8.2 to 1.8.3.
- [Release notes](https://github.com/stretchr/testify/releases)
- [Commits](stretchr/testify@v1.8.2...v1.8.3)

---
updated-dependencies:
- dependency-name: github.com/stretchr/testify
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* fix(mem): Correct capitalization of linux writeback

fixes: shirou#1471

* chore(deps): bump github.com/yusufpapurcu/wmi from 1.2.2 to 1.2.3

Bumps [github.com/yusufpapurcu/wmi](https://github.com/yusufpapurcu/wmi) from 1.2.2 to 1.2.3.
- [Release notes](https://github.com/yusufpapurcu/wmi/releases)
- [Commits](yusufpapurcu/wmi@v1.2.2...v1.2.3)

---
updated-dependencies:
- dependency-name: github.com/yusufpapurcu/wmi
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* [common]: fix potential leak on Sleep.

* chore(deps): bump github.com/stretchr/testify from 1.8.3 to 1.8.4

Bumps [github.com/stretchr/testify](https://github.com/stretchr/testify) from 1.8.3 to 1.8.4.
- [Release notes](https://github.com/stretchr/testify/releases)
- [Commits](stretchr/testify@v1.8.3...v1.8.4)

---
updated-dependencies:
- dependency-name: github.com/stretchr/testify
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* allow to pass context values to override environment variables

* move the Env key out of internal packages

* change to use a typed map per code review

* lint fixes

* remove deprecation comments

* add more env keys

* apply code review

* fix more linting reports

* add(README): add passing context value document

which is introduced by shirou#1439.

* Ref actions by commit SHA in build_test.yml

It's important to make sure the SHA's are from the original repositories and not forks.

For reference:

https://github.com/actions/setup-go/releases/tag/v4.0.1
actions/setup-go@fac708d

https://github.com/actions/checkout/releases/tag/v3.5.2
actions/checkout@8e5e7e5

https://github.com/actions/cache/releases/tag/v3.3.1
actions/cache@88522ab


Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

* Ref actions by commit SHA in labeler.yml

It's important to make sure the SHA's are from the original repositories and not forks.

For reference:

https://github.com/actions/labeler/releases/tag/v4.1.0
actions/labeler@9fcb2c2


Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

* Ref actions by commit SHA in lint.yml

It's important to make sure the SHA's are from the original repositories and not forks.

For reference:

https://github.com/actions/setup-go/releases/tag/v4.0.1
actions/setup-go@fac708d

https://github.com/actions/checkout/releases/tag/v3.5.2
actions/checkout@8e5e7e5

https://github.com/golangci/golangci-lint-action/releases/tag/v3.5.0
golangci/golangci-lint-action@5f1fec7


Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

* Ref actions by commit SHA in release.yml

It's important to make sure the SHA's are from the original repositories and not forks.

For reference:

https://github.com/actions/checkout/releases/tag/v3.5.2
actions/checkout@8e5e7e5


Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

* Ref actions by commit SHA in sbom_generator.yml

It's important to make sure the SHA's are from the original repositories and not forks.

For reference:

https://github.com/actions/checkout/releases/tag/v3.5.2
actions/checkout@8e5e7e5

https://github.com/advanced-security/sbom-generator-action/releases/tag/v0.0.1
advanced-security/sbom-generator-action@375dee8

https://github.com/actions/upload-artifact/releases/tag/v3.1.2
actions/upload-artifact@0b7f8ab


Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

* Ref actions by commit SHA in shellcheck.yml

It's important to make sure the SHA's are from the original repositories and not forks.

For reference:

https://github.com/actions/checkout/releases/tag/v3.5.2
actions/checkout@8e5e7e5

https://github.com/ludeeus/action-shellcheck/releases/tag/2.0.0
ludeeus/action-shellcheck@00cae50


Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

* Ref actions by commit SHA in test.yml

It's important to make sure the SHA's are from the original repositories and not forks.

For reference:

https://github.com/actions/setup-go/releases/tag/v4.0.1
actions/setup-go@fac708d

https://github.com/actions/checkout/releases/tag/v3.5.2
actions/checkout@8e5e7e5

https://github.com/actions/cache/releases/tag/v3.3.1
actions/cache@88522ab


Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>

* chore(deps): bump actions/checkout from 3.5.2 to 3.5.3

Bumps [actions/checkout](https://github.com/actions/checkout) from 3.5.2 to 3.5.3.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@8e5e7e5...c85c95e)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore(deps): bump golangci/golangci-lint-action from 3.5.0 to 3.6.0

Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 3.5.0 to 3.6.0.
- [Release notes](https://github.com/golangci/golangci-lint-action/releases)
- [Commits](golangci/golangci-lint-action@5f1fec7...639cd34)

---
updated-dependencies:
- dependency-name: golangci/golangci-lint-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore(deps): bump golang.org/x/sys from 0.8.0 to 0.9.0

Bumps [golang.org/x/sys](https://github.com/golang/sys) from 0.8.0 to 0.9.0.
- [Commits](golang/sys@v0.8.0...v0.9.0)

---
updated-dependencies:
- dependency-name: golang.org/x/sys
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* cpu_percent and memory_percent update

* chore(deps): bump actions/labeler from 4.1.0 to 4.2.0

Bumps [actions/labeler](https://github.com/actions/labeler) from 4.1.0 to 4.2.0.
- [Release notes](https://github.com/actions/labeler/releases)
- [Commits](actions/labeler@9fcb2c2...0967ca8)

---
updated-dependencies:
- dependency-name: actions/labeler
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* [process] Remove all noisy and useless Test_AllProcesses_X tests

These tests can't fail more than their invidiual counterparts and produce an incredibly verbose output

* chore(deps): bump golang.org/x/sys from 0.9.0 to 0.10.0

Bumps [golang.org/x/sys](https://github.com/golang/sys) from 0.9.0 to 0.10.0.
- [Commits](golang/sys@v0.9.0...v0.10.0)

---
updated-dependencies:
- dependency-name: golang.org/x/sys
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* [process][posix] Realign process.Name() with python psutil to return same value on python3 scripts processes

e2c79a1 started to blindly set the process name to the full path (instead of the basename) of the cmdline exectuable
if the process name from the process comm was truncated on linux. Python psutil never did that, and this is just wrong
for python (or any executable interpreted script) where the process name is not the interpreter binary but the script
itself.

A new test to check process name value against psutil value is added here, which would hopefully catch any potential
future changes in psutil.

Reverts shirou#542

Fixes shirou#1485

* chore(deps): bump actions/labeler from 4.2.0 to 4.3.0

Bumps [actions/labeler](https://github.com/actions/labeler) from 4.2.0 to 4.3.0.
- [Release notes](https://github.com/actions/labeler/releases)
- [Commits](actions/labeler@0967ca8...ac9175f)

---
updated-dependencies:
- dependency-name: actions/labeler
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Enable setting of vendor and related information for all Power versions

* chore(deps): bump golang.org/x/sys from 0.10.0 to 0.11.0

Bumps [golang.org/x/sys](https://github.com/golang/sys) from 0.10.0 to 0.11.0.
- [Commits](golang/sys@v0.10.0...v0.11.0)

---
updated-dependencies:
- dependency-name: golang.org/x/sys
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* mem: linux: fix vmstat field names

The field names are read from /proc/vmstat were capitalized as their
output fields by mistake

* chore(deps): bump github.com/tklauser/go-sysconf from 0.3.11 to 0.3.12

Bumps [github.com/tklauser/go-sysconf](https://github.com/tklauser/go-sysconf) from 0.3.11 to 0.3.12.
- [Release notes](https://github.com/tklauser/go-sysconf/releases)
- [Commits](tklauser/go-sysconf@v0.3.11...v0.3.12)

---
updated-dependencies:
- dependency-name: github.com/tklauser/go-sysconf
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore(deps): bump actions/setup-go from 4.0.1 to 4.1.0

Bumps [actions/setup-go](https://github.com/actions/setup-go) from 4.0.1 to 4.1.0.
- [Release notes](https://github.com/actions/setup-go/releases)
- [Commits](actions/setup-go@fac708d...93397be)

---
updated-dependencies:
- dependency-name: actions/setup-go
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* fix: fixed windows disk package leaks
- fixed goroutine leak in PartitionsWithContext
- closed registry handle in init

* [host][linux]: remove double quote from lsb release info

fix: shirou#1502

* chore(deps): bump golangci/golangci-lint-action from 3.6.0 to 3.7.0

Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 3.6.0 to 3.7.0.
- [Release notes](https://github.com/golangci/golangci-lint-action/releases)
- [Commits](golangci/golangci-lint-action@639cd34...3a91952)

---
updated-dependencies:
- dependency-name: golangci/golangci-lint-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* fix IOCounters() SerialNumber enumeration

* Update disk/disk_linux.go

Co-authored-by: shirou <shirou.faw@gmail.com>

* update comment

* comment cleanup

* chore(deps): bump actions/checkout from 3.5.3 to 3.6.0

Bumps [actions/checkout](https://github.com/actions/checkout) from 3.5.3 to 3.6.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@c85c95e...f43a0e5)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

---------

Signed-off-by: cui fliter <imcusg@gmail.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>
Co-authored-by: cui fliter <imcusg@gmail.com>
Co-authored-by: shirou <shirou.faw@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Josh Powers <powersj@fastmail.com>
Co-authored-by: Antoine Toulme <antoine@lunar-ocean.com>
Co-authored-by: Gabriela Gutierrez <gabigutierrez@google.com>
Co-authored-by: Elfranne <tom.braarup@gmail.com>
Co-authored-by: Lomanic <5020919+Lomanic@users.noreply.github.com>
Co-authored-by: Kishen V <kishen.viswanathan@ibm.com>
Co-authored-by: Hugo Beauzée-Luyssen <hugo.beauzee@datadoghq.com>
Co-authored-by: Ozan HACIBEKİROĞLU <ozanhaci@gmail.com>
Co-authored-by: Greg Dallavalle <greg.dallavalle@lightstep.com>
Co-authored-by: Greg <2582552+gdvalle@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rmfitzpatrick rmfitzpatrick rmfitzpatrick left review comments

@shirou shirou shirou approved these changes

@dloucasfx dloucasfx dloucasfx approved these changes

Assignees
No one assigned
Labels
package:cpu package:disk package:docker package:host package:load package:mem package:net package:process
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Offer a way to avoid using os.GetEnv
4 participants
@atoulme @dloucasfx @shirou @rmfitzpatrick