Skip to content

quick try without host.Info() - #2

Draft
thaJeztah wants to merge 2 commits into
laurazard:auth-device-flowfrom
thaJeztah:without_host_Info
Draft

quick try without host.Info()#2
thaJeztah wants to merge 2 commits into
laurazard:auth-device-flowfrom
thaJeztah:without_host_Info

Conversation

@thaJeztah

Copy link
Copy Markdown

I was, erm, curious where all the extra dependencies originated from 🙈;

  • github.com/tklauser/go-sysconf
  • github.com/tklauser/go-sysconf (number of online CPUs?)
  • github.com/shoenig/go-m1cpu (CPU info for m1 CPUs)
  • github.com/power-devops/perfstat (handling of libperfstat ??
  • github.com/go-ole/go-ole (loading of Windows OLE objects)
  • github.com/yusufpapurcu/wmi (WQL interface to Windows Management Instrumentation (WMI))

And those all look to be related to this single host.Info(); gopsutil/host.Info() is doing a lot of things; https://github.com/shirou/gopsutil/blob/865b8c3f58d6ee4c47dab074fbf3c4b037abfd96/host/host.go#L65-L117

So I went looking "What information do we really need here?"

  • cliVersion - ⚠️ this function is set at compile time; there is no formal definition of the format (and can be set to anything)
  • hostinfo.OS - this is hardcoded to runtime.GOOS in gopsutil/host.Info()
  • hostVersion - this comes from PlatformVersion. Is it a strong requirement to know the version of the OS? On Linux it will be useless without distro. On Windows, it will be "guesswork" (no official API); if really needed, we could also use pkg/parsers/kernel.GetKernelVersion() from the docker code.
  • hostinfo.KernelArch - do we need detailed architecture (including CPU sub-variants), which could even be "off" when running with QEMU emulation; could we use runtime.GOARCH instead?

Can we get rid of this dependency, and replace host.Info() with something more basic?

- A picture of a cute animal (not mandatory but encouraged)

laurazard and others added 2 commits July 3, 2024 17:01
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov-commenter

Copy link
Copy Markdown

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@laurazard
laurazard force-pushed the auth-device-flow branch 3 times, most recently from ed4e811 to 4029dbc Compare July 4, 2024 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants