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

Use VERSION_ID instead of VERSION field for platform version #44

Merged
merged 1 commit into from
Nov 2, 2022

Conversation

ISauve
Copy link

@ISauve ISauve commented Nov 2, 2022

VERSION can include release code name, whereas VERSION_ID is a simplified identifier containing no spaces (i.e. VERSION="17 (Beefy Miracle)", VERSION_ID=17). This makes it much more useful for programmatic usage (specifically, for using version as part of a file path).

If we want the codename in the future, we could always get that from VERSION_CODENAME and return that separately.

Edit: This only affects platform information read from the /etc/os-release file. When platform data is read from /etc/lsb-release (the default location), we get the version information from the DISTRIB_RELEASE field, which is already in this simplified format.

@ISauve ISauve merged commit ad103b6 into dd Nov 2, 2022
@paulcacheux
Copy link

Won't this change impact all other customers of the host.Info() ? This look like a pretty big change

@paulcacheux
Copy link

@ISauve
Copy link
Author

ISauve commented Nov 2, 2022

You're right that the process-agent reads this data (we don't need to worry about other places in the agent as it uses the main fork), but I couldn't find any specific place where it uses the Version field in a way which would be affected by this change. We can check with someone from the processes team to be certain though!

FWIW, this isn't a major change as it just makes os-release return the same information as reading from lsb-release already gives us (and reading from lsb-release is the default behaviour). So all consumers of this data should already expect to receive data in this format.

@ISauve
Copy link
Author

ISauve commented Nov 3, 2022

@mbotarro has taken a look and confirmed that this should be a safe change

@ISauve ISauve deleted the isabelle.sauve/read-version-id branch November 3, 2022 17:40
@brycekahle
Copy link
Member

shirou#1585

@brycekahle brycekahle added the upstreamed change has been merged upstream label Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstreamed change has been merged upstream
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants