Hi yuppie,
Op 09-09-15 om 11:43 schreef yuppie:
Hi Maurits,
Maurits van Rees wrote:
Dependency profiles from metadata.xml that are already applied, are not
applied again. Instead, its upgrade steps, if any, are applied. In code
you can choose the old behavior of always applying the dependencies, by
calling runAllImportStepsFromProfile with always_apply_profiles=True. Or
you can choose to be happy with any applied version and ignore any
upgrade steps, by using upgrade_dependencies=False. Note that these
arguments cannot both be True.
assuming that profiles always depend on the latest versions of specified
profiles is fine. But wouldn't it be better to make upgrading existing
profiles an explicit extra step in the UI? Does it make sense to upgrade
only the dependencies, not other applied profiles?
Basically I am thinking: we used to reapply the entire profile, let's
instead not do nothing at all, but meet halfway: run the upgrade steps.
I guess we could add an extra option in the UI, making use of these new
options. The user already has the option 'Include dependencies' there,
default Yes. An extra option might be 'Apply upgrade steps of already
applied profiles instead of reapplying them completely', with default
Yes. We might then need to make it possible to select all possible
combinations of what I now made possible. Danger is that it gets
confusing for the end user (well, site admin).
Unrelated profiles should be left alone. Possibly a method
'applyAllUpgradeStepsOfAllProfile' could be useful, with a big button on
the Upgrades tab. But not in this pull request.
I would just raise an error if the dependencies are not up to date and
ignoring the problem or running upgrades or re-applying profiles is not
explicitly specified. If only one option is allowed, why not using one
argument? outdated_dependencies=None|ignore|upgrade|reapply
Maybe 'upgrade_strategy=...' It *does* mean one keyword argument less,
which can be nice.
But I can also live with your solution. As long as the behavior doesn't
change if the dependencies were not applied before, I have no objections.
Good.
Note that there is some discussion on github too, but not really about
the above points. See
https://github.com/zopefoundation/Products.GenericSetup/pull/16
Less tricky is the second change:
Be more forgiving when dealing with profile ids with or without profile-
at the start. All functions that accept a profile id argument and only
work when the id does not have this string at the start, will now strip
it off if it is there. For example, getLastVersionForProfile will give
the same answer whether you ask it for the version of profile id foo or
profile-foo.
This doesn't make things clearer to me. IIRC the prefixes were added to
have separate namespaces and an easy way to figure out which kind of
profile we are dealing with.
Do you propose to make these namespaces obsolete in the code? Or only in
some places? How do I know if the profile_id argument requires the
prefix in a specific method?
Basically I want to avoid that GenericSetup says "No, there is no
profile with id X" and silently thinks "You should have asked me for
profile-X instead, I have got that one lying around here just fine."
The 'profile-' and 'snapshot-' prefixes are still used. Most methods
only cared about 'profile-', but some of those expected the stripped id
and others the id with the prefix. I found that frustrating to deal with.
Some methods handle both 'normal' profiles and snapshots. With my
changes, these now have:
- if profile starts with 'snapshot-': do A.
- elif profile starts with 'profile-': do B.
- else: same as 'profile-'
Is that clearer?
--
Maurits van Rees: http://maurits.vanrees.org/
Zest Software: http://zestsoftware.nl
_______________________________________________
Zope-CMF maillist - Zope-CMF@zope.org
https://mail.zope.org/mailman/listinfo/zope-cmf
See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
