Rhel7 fix 1404158 #987
Rhel7 fix 1404158 #987
Conversation
jkonecny12
force-pushed
the
rhel7-fix-1404158
branch
2 times, most recently
from
bc2e2fd
to
02c7171
Compare
There was a problem hiding this comment.
Small suggestion for the last commit:
"However if user visits the Software
spoke it will erase kickstart settings and use user settings."
->
"However if user visits the Software spoke the settings from kickstart (environment & groups) will be replaced by what the user selects in the spoke. Individual package selection from kickstart should be unaffected."
These aside looks really good to me - thanks for looking into this! :)
pyanaconda/ui/gui/spokes/software.py
Outdated
| @@ -156,7 +160,12 @@ def _payload_error(self): | |||
| hubQ.send_message(self.__class__.__name__, payloadMgr.error) | |||
|
|
|||
| def _apply(self): | |||
| if self.environment and not (flags.automatedInstall and self.data.packages.seen): | |||
| # Environment have to be set by GUI but not by kickstart | |||
There was a problem hiding this comment.
Rewording suggestion:
Environment needs to be set during a GUI installation, but is not required for a kickstart install (even partial)
pyanaconda/ui/gui/spokes/software.py
Outdated
| @@ -464,6 +461,19 @@ def _get_selected_addons(self): | |||
|
|
|||
| return retval | |||
|
|
|||
| def _mark_addon_selection(self, grpid, active): | |||
There was a problem hiding this comment.
Maybe selected instead of active? There is no gui in this function, so active does not make much sense.
There was a problem hiding this comment.
Sounds better to me. Fixed.
pyanaconda/packaging/yumpayload.py
Outdated
| """ | ||
| groups = self._yumGroups | ||
| if not groups: | ||
| return "" |
There was a problem hiding this comment.
This will cause that self._addonStates[""] will be set several times to self._ADDON_SELECTED. Maybe it would be better to raise an exception, since we catch PayloadError anyway when selectedGroupsIDs is called.
There was a problem hiding this comment.
I want to have the same behavior as groupDescription() method has, this method is returning (groupid, groupid).
It would be unexpected behavior when one method is returning something and second similar method below raising an exception.
Also catching an exception takes more performance than rewriting value in a dictionary.
There was a problem hiding this comment.
Ok, but we could at least return None and check the id before we modify self._addonStates.
There was a problem hiding this comment.
I don't want to see return None. I think it is straight way to hard debugging because you never know where this will propagate in future, and None is acting as normal value in many ways.
However you are right that the empty string is not the right solution here. So last man standing, use an exception. I don't like it here either for reasons above but it is smallest evil right now. Therefore I'm going to change this to exception.
There was a problem hiding this comment.
It's an unexpected/exceptional case, right?
Removed selectedGroups which is not helpful and use addonStates dictionary instead. The addonStates variable has 3 states, SELECTED, DESELECTED and DEFAULT. When you change the state to the environment default, it will return state in the addonStates variable to DEFAULT. Remove code for removing groups from selectedGroups array when changing environment; this is not required anymore when the state is read from buttons. Groups marked for installation are read from GTK now. Resolves: rhbz#1404158 Reported-by: Vit Ry <Frodox@zoho.com>
Removed excludedGroups which was always empty array. Removed _parseEnvironments() method which wasn't called anywhere and it's not required by the code. Related: rhbz#1404158
jkonecny12
force-pushed
the
rhel7-fix-1404158
branch
from
02c7171
to
9398f9e
Compare
|
UPDATED |
When groups are selected in %packages section in a kickstart file, mark these groups in the Software spoke. However if the user visits the Software spoke the settings from kickstart (environment & groups) will be replaced by what the user selects in the spoke. Individual package selection from kickstart should be unaffected. Kickstart file without user interaction is the only option how to install groups and packages which are not inside of the selected environment. Related: rhbz#1404158
jkonecny12
force-pushed
the
rhel7-fix-1404158
branch
from
9398f9e
to
59bf385
Compare
|
Is it OK now @poncovka ? |
Fix Software spoke.