Let linuxmusterTools import without a configured linuxmuster.net - #26
Open
TomlDev wants to merge 1 commit into
Open
Let linuxmusterTools import without a configured linuxmuster.net#26TomlDev wants to merge 1 commit into
TomlDev wants to merge 1 commit into
Conversation
lmnconfig/samba.py ran `/usr/bin/net conf list` as an unguarded module-level side effect, and lmnconfig/sophomorix.py read the ROLE_USER section of a sophomorix.ini that need not exist. Importing anything from linuxmusterTools.linbo reaches both through linbo.config -> devices -> lmnconfig, so on any machine without a configured linuxmuster.net the whole package failed to import with FileNotFoundError or KeyError. That is what stops the pytests from running anywhere but a server: the lmnconfig suite does not fail, it fails to collect. test_samba.py says so in its own module docstring, and no workflow runs the tests. Both sites now behave like the code directly next to them, which already guarded the same class of problem: the smb.conf read in samba.py sits in a try/except, and SchoolConfig checks os.path.isfile before opening. A missing or failing `net` leaves SHARES_LIST and DFS empty and logs the reason; a missing sophomorix.ini leaves the role lists empty and warns. Behaviour on a configured server is unchanged. The 38 existing lmnconfig tests pass, and three cover the new paths.
Member
|
Hello @TomlDev,
Yes, that's a wanted behavior: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
linuxmusterToolscannot be imported on a machine that has no configured linuxmuster.net, because two module-level side effects assume one is present:lmnconfig/samba.pyruns/usr/bin/net conf listat import time, unguarded —FileNotFoundErrorwhere samba is not installed.lmnconfig/sophomorix.pyreads theROLE_USERsection of/usr/share/sophomorix/devel/sophomorix.ini—ConfigParser.read()silently ignores the missing file, so the lookup raisesKeyError.Importing anything from
linuxmusterTools.linboreaches both, throughlinbo.config→devices→lmnconfig.The visible consequence is the test suite:
linuxmusterTools/lmnconfig/pytests/does not fail off-server, it fails to collect.test_samba.pyalready records this in its module docstring — "computed as a module-level side effect at import time … not re-testable/mockable per test case, so it is intentionally left untested here". Since no workflow runs the tests, this is only visible to someone trying to run them on a workstation.What changed
Both sites now behave like the code directly beside them, which already guards the same class of problem — the
smb.confread insamba.pysits in atry/except, andSchoolConfigchecksos.path.isfilebefore opening:netleavesSHARES_LISTempty andDFSempty, and logs the reason at error level, in the wording the neighbouring handler already uses.sophomorix.iniwarns, anduserrolefalls back to an empty list via.get().Behaviour on a configured server is unchanged — both paths only trigger where the command or the file is absent.
Tests
The 38 existing
lmnconfigtests pass. Three new ones cover the added paths: import surviving a missingnetbinary, import surviving anetcall that exits non-zero, andSophomorixIniwith noROLE_USERsection.Reverting the two source files while keeping the tests reproduces the original failure — the suite stops at collection with
FileNotFoundError: '/usr/bin/net', which is the point of the change.I did not touch the version; that is the maintainers'
bumpversionstep.