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

Fstab handling moves to blivet #5793

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

japokorn
Copy link
Contributor

@japokorn japokorn commented Jul 31, 2024

Anaconda handling fstab is something that should be done by blivet. This moves the fstab logic into the blivet. Crypttab and blkidtab handling is not supported by blivet yet so it remains mostly unchanged (for now)

Added unit_tests

@pep8speaks
Copy link

pep8speaks commented Jul 31, 2024

Hello @japokorn! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-08-08 10:10:58 UTC

@github-actions github-actions bot added the f41 label Jul 31, 2024
Anaconda handling fstab is something that should be done by blivet.
This moves the fstab logic into the blivet. Crypttab and blkidtab
handling is not supported by blivet yet so it remains mostly unchanged
(for now)

Added unit_tests
Copy link
Contributor

@vojtechtrefny vojtechtrefny left a comment

Choose a reason for hiding this comment

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

The tests are not very happy about this right now, but in general it looks good to me, good to move this stuff out of Anaconda.

@@ -20,6 +20,7 @@
import time

from blivet import blockdev
from blivet.fstab import FSTabManager
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also bump required version of blivet in spec to the version adding fstab support.

self.assertTrue(test_dev in devices)

print("MYDEBUG: %s" % mounts)
print("MYDEBUG: %s" % devices)
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably shouldn't be here :-)

@vojtechtrefny vojtechtrefny added the blocked Don't merge this pull request! label Aug 1, 2024
@vojtechtrefny
Copy link
Contributor

Marking as blocked, because we'll need some changes in Blivet that are not yet released, see storaged-project/blivet#1263

- removed leftover debug messages from test_fsset.py
- removed unused imports from fsset.py
- changed variable's names to correct values in root.py
- increased required blivet version in spec file
Copy link

This PR is stale because it has been open 60 days with no activity.
Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the stale label Oct 19, 2024
@KKoukiou
Copy link
Contributor

KKoukiou commented Nov 4, 2024

@japokorn do you think you can find some time to finish the work here ?

@KKoukiou KKoukiou added f42 Fedora 42 and removed stale f41 labels Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Don't merge this pull request! f42 Fedora 42
Development

Successfully merging this pull request may close these issues.

4 participants