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

Undo deprecation warning for len(batch) #3176

Closed
mauritsvanrees opened this issue Sep 16, 2020 · 1 comment
Closed

Undo deprecation warning for len(batch) #3176

mauritsvanrees opened this issue Sep 16, 2020 · 1 comment

Comments

@mauritsvanrees
Copy link
Sponsor Member

In Plone 5.2 you get lots of deprecation warnings like this, in real-world use, and for easy of reproduction also when running for example this test (edited to filter out other warnings):

$ bin/test -s Products.CMFPlone -t pwreset_browser.rst
...
  Running:
    1/1 (100.0%)/Users/maurits/community/plone-coredev/py3/var/cache/f8b4063628323927100f9f247fe9c029.py:460: DeprecationWarning: Using len() is deprecated. Use the `length` attribute for the size of the current page, which is what we return now. Use the `sequence_length` attribute for the size of the entire sequence. 
  if __condition:
/Users/maurits/community/plone-coredev/py3/src/Zope/src/Products/PageTemplates/engine.py:74: DeprecationWarning: Using len() is deprecated. Use the `length` attribute for the size of the current page, which is what we return now. Use the `sequence_length` attribute for the size of the entire sequence. 
  iterable = list(iterable) if iterable is not None else ()
/Users/maurits/community/plone-coredev/py3/var/cache/0a229eac7e57a317bdd047ef739d4daf.py:137: DeprecationWarning: Using len() is deprecated. Use the `length` attribute for the size of the current page, which is what we return now. Use the `sequence_length` attribute for the size of the entire sequence. 
  if __condition:
...
/Users/maurits/community/plone-coredev/py3/src/Zope/src/Products/PageTemplates/Expressions.py:253: DeprecationWarning: Using len() is deprecated. Use the `length` attribute for the size of the current page, which is what we return now. Use the `sequence_length` attribute for the size of the entire sequence. 
  return bool(value)
/Users/maurits/shared-eggs/cp37m/zope.deprecation-4.4.0-py3.7.egg/zope/deprecation/deprecation.py:88: DeprecationWarning: isDefaultPage is deprecated. Import from Products.CMFPlone.defaultpage instead
  name)
/Users/maurits/community/plone-coredev/py3/var/cache/16a60114d1d42d8280752740f6a1c9e7.py:1871: DeprecationWarning: Using len() is deprecated. Use the `length` attribute for the size of the current page, which is what we return now. Use the `sequence_length` attribute for the size of the entire sequence. 
  if __condition:

Those are actually two different warnings. The second one is due to a missing __bool__ and will be fixed with my PR #3175. The first one is from code in Products.PageTemplates that handle iteration over a repeat. If you have a batch and in a template you do tal:repeat="item batch", then this code calls len(batch) to get the length of repeat. Actually, list(batch) already does this, when I look at the traceback.

The call to len(batch) is deprecated in CMFPlone. This was first done in 2011, for Plone 4.2, in commit 84bdce6. It advocated using the pagesize attribute at first. In 2015, for Plone 5.0, in commit a66ed54 I changed this to recommend length or sequence_length, depending on what you wanted.

But as you can see, Products.PageTemplates happily calls len(batch), and this happens everytime someone uses a batch in a template. This makes the deprecation useless by now, I think: people cannot avoid this warning.

An option could be to change this in Products.PageTemplates (and possibly chameleon as well, but I don't want to think about that just now), something like:

if hasattr(iterable, "length"):
    length = iterable.length
    if callable(length):
        length = length()
else:
    length = len(iterable)

But I don't think it makes sense to have special code like this for PloneBatch in Products.PageTemplates.

So a proposal: let's undo this deprecation. In other words: remove these lines. (But keep them as a comment probably.) This would be for Plone 5.2.3.

@plone/framework-team Thoughts?

@ale-rt
Copy link
Member

ale-rt commented Sep 16, 2020

I agree with undoing the deprecation. It makes sense that the length of the batch is the number of batched element.

mauritsvanrees added a commit that referenced this issue Sep 28, 2020
The deprecation warning is unvoidable with current `Products.PageTemplates` code.
Fixes #3176
jensens pushed a commit that referenced this issue Sep 28, 2020
The deprecation warning is unvoidable with current `Products.PageTemplates` code.
Fixes #3176

(cherry picked from commit 52a3267)
ale-rt added a commit that referenced this issue Feb 13, 2021
* Fix TTW Bundle compilation broken.
Fixes #2969

* Add plone.staticresources to list of addons which are automatically upgraded if upgrade steps are available.

* Fix #1318: Always install default content types on Plone site creation (#2971)

* Fix #1318: Always install default content types on Plone site creation

* Update changelog entry

* Correctly fire events when user auto login after the password has been reset (#2994)

* Test if events fired after auto login have the correct principal

* At this point the user is not logged in yet, so we get it from the membership_tool.

* Update changelog

* Remove adapter registration for IUserLoggedInEvent

* Fix unregistering of adapter

* Deal with cases where username/email were not the userid

* Create FUNDING.yml

* Improve tests for the workflow tool method listWFStatesByTitle

Refs #3032

* require Zope wsgi extra (because of Paste) depending on Python version

* add changelog entry

* Make setup.py reflect to be Plone 6 and Py3 only

* Include changelog of 5.2.1.

Removed related news snippets.
There are now no differences between master and 5.2.x, except basically in setup.py.

[ci skip]

* Password strength checks were not always checked

Include Hotfix 20200121 with tests

* Merge isURLInPortal patch from Hotfix20200121.

The isURLInPortal check that is done to avoid linking to an external
site could be tricked into accepting malicious links.

* Fix typo

causes double translation text. Same message is on line 350

* enable custom format string i18n/l10n

* fix/add suggestions from review

* Use time.process_time always.

Available in all relevant Python 3 versions.

* add viewlet manager below content description

* structure cleanup

* fix card markup for footer

* fix footer and colophon card classes

* Add changelog entry

* Fix TTW Bundle compilation broken.
Fixes #2969

* Add plone.staticresources to list of addons which are automatically upgraded if upgrade steps are available.

* Fix #1318: Always install default content types on Plone site creation (#2971)

* Fix #1318: Always install default content types on Plone site creation

* Update changelog entry

* Correctly fire events when user auto login after the password has been reset (#2994)

* Test if events fired after auto login have the correct principal

* At this point the user is not logged in yet, so we get it from the membership_tool.

* Update changelog

* Remove adapter registration for IUserLoggedInEvent

* Fix unregistering of adapter

* Deal with cases where username/email were not the userid

* changelog

* Create FUNDING.yml

* require Zope wsgi extra (because of Paste) depending on Python version

* add changelog entry

* Make setup.py reflect to be Plone 6 and Py3 only

* Include changelog of 5.2.1.

Removed related news snippets.
There are now no differences between master and 5.2.x, except basically in setup.py.

[ci skip]

* Password strength checks were not always checked

Include Hotfix 20200121 with tests

* Merge isURLInPortal patch from Hotfix20200121.

The isURLInPortal check that is done to avoid linking to an external
site could be tricked into accepting malicious links.

* Improve tests for the workflow tool method listWFStatesByTitle

Refs #3032

* Fix typo

causes double translation text. Same message is on line 350

* enable custom format string i18n/l10n

* fix/add suggestions from review

* Use time.process_time always.

Available in all relevant Python 3 versions.

* Add custom.css resource after diazo bundle

* Add changelog entry

* Add issue number to changelog entry

* set GS version to 6000

* Add zcml-condition plone-60 for conditional configuration.

* update HTMLFilter defaults to enable TinyMCE layout features

* Removed a few news snippets that are already in latest 5.2.2 rc.

* Add links to official resources

* The http://plone.org/products/plone page is no longer current.

* Fix rebase aftermath: Missing merge

* Merge isURLInPortal patch from Hotfix20200121.

The isURLInPortal check that is done to avoid linking to an external
site could be tricked into accepting malicious links.

* add viewlet manager below content description

* structure cleanup

* fix card markup for footer

* fix footer and colophon card classes

* simplify login form and fix login action pattern-options

* Styled content and usergroups in Site Setup

* fix primary button css class

* fix tests to match bootstrap form markup

* fix broken/incomplete merge from rebase

* fix broken/incomplete merge from rebase (2)

* Fix issue with @@search view when filtering by creation date

(cherry picked from commit 4dbed04)

* remove six - we are Python 3 only

* Remove portal_utf8 and it twin utf8_portal from utils and PloneTool since its never used nowhere.

* Renamed wrongly named news snippets.

[ci skip]

* Removed folder_publish.cpy script.

Moved it to a new browser view in plone.app.content.
Removed deprecated transitionObjectsByPaths.
It was only used by this script, and I have now put it in the view.

* No longer consider calling len(batch) as deprecated.

The deprecation warning is unvoidable with current `Products.PageTemplates` code.
Fixes #3176

(cherry picked from commit 52a3267)

* find . -name "*.py" |xargs pyupgrade --py36-plus

* Fixed news snippet extension.

[ci skip]

* Removed our CMFQuickInstallerTool code completely.

* Removed QuickInstallerTool.py

With the Products.CMFQuickInstallerTool package gone,
the bare bones fallback in this file did not work correctly.
You get an error like this when viewing the Plone Site in the ZMI,
or even just loading the root ZMI:

```
2020-04-24 23:29:39,113 ERROR   [ZODB.Connection:809][waitress] Couldn't load state for Products.CMFPlone.Portal.PloneSite 0x9f8a
Traceback (most recent call last):
  File "/Users/maurits/shared-eggs/cp37m/ZODB-5.5.1-py3.7.egg/ZODB/Connection.py", line 795, in setstate
    self._reader.setGhostState(obj, p)
  File "/Users/maurits/shared-eggs/cp37m/ZODB-5.5.1-py3.7.egg/ZODB/serialize.py", line 637, in setGhostState
    state = self.getState(pickle)
  File "/Users/maurits/shared-eggs/cp37m/ZODB-5.5.1-py3.7.egg/ZODB/serialize.py", line 630, in getState
    return unpickler.load()
  File "/Users/maurits/shared-eggs/cp37m/ZODB-5.5.1-py3.7.egg/ZODB/serialize.py", line 492, in _persistent_load
    return self.load_persistent(*reference)
  File "/Users/maurits/shared-eggs/cp37m/ZODB-5.5.1-py3.7.egg/ZODB/serialize.py", line 535, in load_persistent
    self._cache.new_ghost(oid, obj)
TypeError: Cache values must be persistent objects.
```

I am creating an alias in plone.app.upgrade.

* Removed outdated test for QuickInstallerTool.

* Removed unused quickinstaller test zcml files.

These were already copied to controlpanel/tests/ and adapted.

* On Python 3 in testMigrationTool only test upgrade from Plone 5.2.

You cannot upgrade a Plone 4.0 or 5.1 site to 5.2 or higher when you are on Python 3.
You must first migrate to 5.2 with Python 2.7.
Also, this avoids a test failure on 6.0 because the 5.0alpha upgrade calls qi.isProductInstalled, which has been removed.

* Remove Python 2 code from testMigrationTool.py.

* Merge isURLInPortal patch from Hotfix20200121.

The isURLInPortal check that is done to avoid linking to an external
site could be tricked into accepting malicious links.

* add viewlet manager below content description

* structure cleanup

* fix card markup for footer

* fix footer and colophon card classes

* simplify login form and fix login action pattern-options

* Styled content and usergroups in Site Setup

* fix primary button css class

* fix tests to match bootstrap form markup

* fix broken/incomplete merge from rebase

* fix broken/incomplete merge from rebase (2)

* add test rendering as view, add basic example

* bootstrap markup update for login

* add icon retriever as view

* add changelog entry

* changed due to feedback

* re-add init method, add fallback for icon lookup

* update registration, refactor icons.py, add tests

* shorten icon prefix

* use iconresolver as name

* update markup

* sync ajax_main_template defines with main_template

* fix outdated pull classes

* lookup fallback trial for mimetypes

* simplify debugging

* better error handling with non-existig icons

* catch specific Exception

* bs5 form updates

* overhaul the admin-ui templates to have a modern bootstrap 5 look

* forgotten test condition removed

* ensure installation of "default_extension_profiles"

* update markup for bootstrap

* updated viewlets for barceloneta-lts

* remove deprecated old controlpanel

* get rid of kss traces

* better var name

* use plone icon names

* move image handling controlpanel to default profile

* use icons from icon resolver

* use icons from resolver

* fallback for icon_expr with urls

* add icons from icon resolver

* cleanup

* update button classes for bootstrap

* update classes for bootstrap

* add SVG modifier

* liberated prefs_main_temaplte from skins

* turn configlet portlets into menu

* manage spaces in add site

* update icon fallback for explicit icon names

* update controlpanel icons to explicit icon names

* fix ajax load: Compilation failed b/c of typo

* controlpanel icon tweaks

* bs5-b1 update - prefixed data attributes

* Cool URLs do not change

* bootstrap b1 changes

* bootstrap b1 changes

* fix alert margin

* Display the path to have a better hint of were the site is

* Fix problem to remove username and password if there was already one set

(cherry picked from commit 0499b63)

* fixes in usergroups controlpanel

* fixes in group member administration

* again some polishing in user/groups admin

* and again some polishing in user/groups admin

* another alert statusmessage

* always allow to set timezone

* fix action controlpanel robot tests

* use different text to check, since breadcrumbs have changed

* fix robot test: no documentFirstheading in this place

* fix checkboxes in usergroups

* register test rendering page as test_rendering and test-rendering

* fix selector for warning alert

* Start fixing search page

* fix search.js -> new selectors

* ajax pagination and sane result numbering

* fix cite tag

* fix searchterm in title

* convert tal:attributes expressions

* reset batch start when filtering

* one more batch fix

* fix controlpanel tests

* remove outdated test - portal_skins doesn't exist anymore

* fix resource url

* fix authenticator test

* fix selector

* add Sleep 0.5s

* fix usergroups robot tests

* refactor usergroups settings description

* fix selector in security_controlpanel test

* Link modal has no input field "title"

* fix overlay tests

* Fix create new action in action control panel

* update icon_expr for actions

Co-authored-by: Johannes Raggam <thetetet@gmail.com>
Co-authored-by: Érico Andrei <ericof@gmail.com>
Co-authored-by: Thomas Massmann <thomas.massmann@it-spir.it>
Co-authored-by: Jens W. Klein <jk@kleinundpartner.at>
Co-authored-by: T. Kim Nguyen <tkimnguyen@users.noreply.github.com>
Co-authored-by: ale-rt <alessandro.pisa@gmail.com>
Co-authored-by: tschorr <t_schorr@gmx.de>
Co-authored-by: Maurits van Rees <maurits@vanrees.org>
Co-authored-by: Maurits van Rees <m.van.rees@zestsoftware.nl>
Co-authored-by: Fred van Dijk <fredvd@gmail.com>
Co-authored-by: agitator <agitator@users.noreply.github.com>
Co-authored-by: Stefan Antonelli <stefan.antonelli@operun.de>
Co-authored-by: Peter Holzer <peter.holzer@agitator.com>
Co-authored-by: MrTango <md@derico.de>
Co-authored-by: Kristin Kuche <kuche@hrz.uni-marburg.de>
Co-authored-by: Mikel Larreategi <mlarreategi@codesyntax.com>
Co-authored-by: Fulvio Casali <fulviocasali@gmail.com>
Co-authored-by: Nils Hofer <niho2001@gmail.com>
Co-authored-by: Franco Pellegrini <frapell@gmail.com>
Co-authored-by: Robert Kuzma <robert@balavec.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants