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

console_conf: use the correct /run directory #1869

Closed
wants to merge 1 commit into from
Closed

console_conf: use the correct /run directory #1869

wants to merge 1 commit into from

Conversation

kubiko
Copy link
Contributor

@kubiko kubiko commented Nov 7, 2023 •
edited
Loading

We should use one /run/console_conf directory.
As this is defined dynamically based on the package name, align
other places where /run/console-conf is used and use /run/console_conf instead, to avoid cluttering of /run directory.
This also complicated strict confinement.

@kubiko kubiko force-pushed the console-conf-use-one-run-dir branch from 5cad4f0 to 60af7ce Compare November 7, 2023 23:54
@kubiko kubiko changed the title subiquitycore: core: use the correct directory to track subiquity state console_conf: use the correct /run directory Nov 7, 2023
dbungert
dbungert reviewed Nov 15, 2023
View reviewed changes
Copy link
Collaborator

@dbungert dbungert left a comment

Choose a reason for hiding this comment

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

Can we look this up from SNAP_NAME or something?

@kubiko
Copy link
Contributor Author

kubiko commented Jan 4, 2024 •
edited
Loading

@dbungert yes this could easily be reworked to use snap name
I will prepare this
The main reason why I did not use SNAP_NAME is that we need to use console_conf vs console-conf where later is permitted snap name.
But console_conf is a package name, which has been used at different place as a way to determine directory name.
This is the reason we ended up using both paths /run/console_conf as well as /run/console-conf

@kubiko kubiko force-pushed the console-conf-use-one-run-dir branch from 60af7ce to c220721 Compare January 6, 2024 02:23
@kubiko
Copy link
Contributor Author

kubiko commented Jan 6, 2024

@dbungert I have found from where /run/console_conf was coming from
this is set here: core.py
So the culprit was actually how the project name was set.
So for now, to keep consistency I would propose to align the project name here with what we use in other parts of the code as a name to form /run dir.

@kubiko
console_conf: use 'console-conf' as project name
a8a7e80 
'project' value is used to construct /run/<project> path
Other parts are already using 'console-conf', prefer this syntax.
In the future we might consider using SNAP_NAME, and this '_' are
not permitted as part of the snap name.

Signed-off-by: Ondrej Kubik <[email protected]>
@kubiko kubiko force-pushed the console-conf-use-one-run-dir branch from c220721 to a8a7e80 Compare January 17, 2024 17:46
@kubiko
Copy link
Contributor Author

kubiko commented Jan 17, 2024

rebasing on main, to make clean diff

@bboozzoo bboozzoo mentioned this pull request Feb 6, 2024
Merged
@bboozzoo
Copy link
Contributor

bboozzoo commented Feb 6, 2024

@dbungert I've opened #1904 which includes the patch from @kubiko with some tweaks and a unit test for the identity controller. Please have a look.

@kubiko
Copy link
Contributor Author

kubiko commented Feb 8, 2024

closing as this has been now superseded by #1904

@kubiko kubiko closed this Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbungert dbungert dbungert left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kubiko @bboozzoo @dbungert