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

Plone 5.1 #298

Closed
tisto opened this Issue Mar 24, 2017 · 15 comments

Comments

Projects
None yet
4 participants
@tisto
Member

tisto commented Mar 24, 2017

Make sure plone.restapi works with Plone 5.1. Make sure all tests and the travis build passes. cc @tomgross

@tisto

This comment has been minimized.

Show comment
Hide comment
@tisto

tisto Mar 24, 2017

Member

Travis build is in place: https://travis-ci.org/plone/plone.restapi/jobs/214588779 Buildout fails though:

Setting socket time out to 3 seconds.
Getting distribution for 'mr.developer==1.35'.
Got mr.developer 1.35.
Getting distribution for 'appdirs==1.4.2'.
zip_safe flag not set; analyzing archive contents...
While:
  Installing.
  Loading extensions.
  Getting distribution for 'appdirs==1.4.2'.

An internal error occurred due to a bug in either zc.buildout or in a
recipe being used:
Traceback (most recent call last):
  File "/home/travis/build/plone/plone.restapi/local/lib/python2.7/site-packages/zc/buildout/buildout.py", line 1982, in main
    getattr(buildout, command)(args)
  File "/home/travis/build/plone/plone.restapi/local/lib/python2.7/site-packages/zc/buildout/buildout.py", line 509, in install
    self._load_extensions()
  File "/home/travis/build/plone/plone.restapi/local/lib/python2.7/site-packages/zc/buildout/buildout.py", line 1035, in _load_extensions
    newest=self.newest, allow_hosts=self._allow_hosts)
  File "/home/travis/build/plone/plone.restapi/local/lib/python2.7/site-packages/zc/buildout/easy_install.py", line 913, in install
    return installer.install(specs, working_set)
  File "/home/travis/build/plone/plone.restapi/local/lib/python2.7/site-packages/zc/buildout/easy_install.py", line 714, in install
    for dist in self._get_dist(req, ws):
  File "/home/travis/build/plone/plone.restapi/local/lib/python2.7/site-packages/zc/buildout/easy_install.py", line 563, in _get_dist
    dists = [_move_to_eggs_dir_and_compile(dist, self._dest)]
  File "/home/travis/build/plone/plone.restapi/local/lib/python2.7/site-packages/zc/buildout/easy_install.py", line 1730, in _move_to_eggs_dir_and_compile
    assert newdist is not None  # newloc above is missing our dist?!
AssertionError

travis_time:end:03df3462:start=1490349970611469701,finish=1490349973945631307,duration=3334161606
�[0K
�[31;1mThe command "bin/buildout -N -t 3 -c travis.cfg" failed and exited with 1 during .�[0m

Your build has been stopped.
Member

tisto commented Mar 24, 2017

Travis build is in place: https://travis-ci.org/plone/plone.restapi/jobs/214588779 Buildout fails though:

Setting socket time out to 3 seconds.
Getting distribution for 'mr.developer==1.35'.
Got mr.developer 1.35.
Getting distribution for 'appdirs==1.4.2'.
zip_safe flag not set; analyzing archive contents...
While:
  Installing.
  Loading extensions.
  Getting distribution for 'appdirs==1.4.2'.

An internal error occurred due to a bug in either zc.buildout or in a
recipe being used:
Traceback (most recent call last):
  File "/home/travis/build/plone/plone.restapi/local/lib/python2.7/site-packages/zc/buildout/buildout.py", line 1982, in main
    getattr(buildout, command)(args)
  File "/home/travis/build/plone/plone.restapi/local/lib/python2.7/site-packages/zc/buildout/buildout.py", line 509, in install
    self._load_extensions()
  File "/home/travis/build/plone/plone.restapi/local/lib/python2.7/site-packages/zc/buildout/buildout.py", line 1035, in _load_extensions
    newest=self.newest, allow_hosts=self._allow_hosts)
  File "/home/travis/build/plone/plone.restapi/local/lib/python2.7/site-packages/zc/buildout/easy_install.py", line 913, in install
    return installer.install(specs, working_set)
  File "/home/travis/build/plone/plone.restapi/local/lib/python2.7/site-packages/zc/buildout/easy_install.py", line 714, in install
    for dist in self._get_dist(req, ws):
  File "/home/travis/build/plone/plone.restapi/local/lib/python2.7/site-packages/zc/buildout/easy_install.py", line 563, in _get_dist
    dists = [_move_to_eggs_dir_and_compile(dist, self._dest)]
  File "/home/travis/build/plone/plone.restapi/local/lib/python2.7/site-packages/zc/buildout/easy_install.py", line 1730, in _move_to_eggs_dir_and_compile
    assert newdist is not None  # newloc above is missing our dist?!
AssertionError

travis_time:end:03df3462:start=1490349970611469701,finish=1490349973945631307,duration=3334161606
�[0K
�[31;1mThe command "bin/buildout -N -t 3 -c travis.cfg" failed and exited with 1 during .�[0m

Your build has been stopped.
@tisto

This comment has been minimized.

Show comment
Hide comment
@tisto

tisto Mar 24, 2017

Member

I managed to fix the buildout. Though, it seems all Plone versions we are testing use different test fixtures (p.a.contenttypes):

https://travis-ci.org/plone/plone.restapi/jobs/214614049

I don't know how we can handle this without putting a lot of effort into it. See: plone/plone.app.contenttypes@f26100b

Member

tisto commented Mar 24, 2017

I managed to fix the buildout. Though, it seems all Plone versions we are testing use different test fixtures (p.a.contenttypes):

https://travis-ci.org/plone/plone.restapi/jobs/214614049

I don't know how we can handle this without putting a lot of effort into it. See: plone/plone.app.contenttypes@f26100b

@tisto

This comment has been minimized.

Show comment
Hide comment
@tisto

tisto Mar 24, 2017

Member

FYI: If anybody wants to take over this, I'd appreciate it. I will focus on other tasks for now...

Member

tisto commented Mar 24, 2017

FYI: If anybody wants to take over this, I'd appreciate it. I will focus on other tasks for now...

@tisto

This comment has been minimized.

Show comment
Hide comment
@tisto

tisto Mar 25, 2017

Member

I made buildout work and tests pass for all Plone versions:

https://travis-ci.org/plone/plone.restapi/builds/214931199

Though, the differences in the testing layers of plone.app.contenttypes and changes in plone.app.event kill our json output script (and will make it hard to impossible for integrators to write proper tests across different Plone versions).

See the Plone 5.1 trace for details about the differences in the testing layers:

https://travis-ci.org/plone/plone.restapi/jobs/214931202

cc @lukasgraf @pbauer @thet

Member

tisto commented Mar 25, 2017

I made buildout work and tests pass for all Plone versions:

https://travis-ci.org/plone/plone.restapi/builds/214931199

Though, the differences in the testing layers of plone.app.contenttypes and changes in plone.app.event kill our json output script (and will make it hard to impossible for integrators to write proper tests across different Plone versions).

See the Plone 5.1 trace for details about the differences in the testing layers:

https://travis-ci.org/plone/plone.restapi/jobs/214931202

cc @lukasgraf @pbauer @thet

@tisto

This comment has been minimized.

Show comment
Hide comment
@tisto

tisto Mar 27, 2017

Member

Here is a detailed list of the differences between the test layers of Plone 5.0 and Plone 5.1:

   "creators": [
-    "test_user_1_", 
-    "admin"
+    "test_user_1_"
+  ], 

@pbauer this is due to your removal of the admin use in the p.a.contenttypes test fixture. I would suggest that we remove that user for all branches of p.a.contenttypes. This could potentially break test setups. Though, since we already released p.a.contenttypes with the admin-user removal there is no way back on this.

+  "customViewFields": [
+    "Title", 
+    "Creator", 
+    "Type", 
+    "ModificationDate"
   ], 
-  "customViewFields": [], 

Not sure why the customViewFields differ.

   "description": "This is an event", 
   "effective": null, 
-  "end": null, 
+  "end": "2013-01-01T12:00:00", 
   "event_url": null, 
   "exclude_from_nav": false, 
   "expires": null, 
@@ -27,7 +26,7 @@ Content-Type: application/json
   "language": "", 
   "location": null, 
   "modified": "2016-01-21T03:24:11+00:00", 
-  "open_end": null, 
+  "open_end": false, 
   "parent": {
     "@id": "http://localhost:55001/plone", 
     "@type": "Plone Site", 
@@ -42,11 +41,10 @@ Content-Type: application/json
     "@id": "http://localhost:55001/plone/event/@sharing", 
     "title": "Sharing"
   }, 
-  "start": null, 
+  "start": "2013-01-01T10:00:00", 
   "subjects": [], 
   "sync_uid": null, 
   "text": null, 
-  "timezone": null, 
   "title": "Event", 
-  "whole_day": null
+  "whole_day": false
 }

@thet it seems the implementation of plone.app.event has been changed considerably between Plone 5.0 and 5.1. Could you please elaborate the reasoning behind this?

+      "description": "This name will be displayed in the URL.", 
+      "title": "Short name", 
+      "type": "string"
+    }, 

It seems the default behavior of the id field has been changed in Plone 5.1. @plone/framework-team any idea?

   {
-    "@id": "http://localhost:55001/plone/@vocabularies/Group Ids", 
-    "title": "Group Ids"
+    "@id": "http://localhost:55001/plone/@vocabularies/plone.app.vocabularies.Users", 
+    "title": "plone.app.vocabularies.Users"
   }, 

Then we have tons of changes in the vocabularies. @plone/framework-team, any clues?

Member

tisto commented Mar 27, 2017

Here is a detailed list of the differences between the test layers of Plone 5.0 and Plone 5.1:

   "creators": [
-    "test_user_1_", 
-    "admin"
+    "test_user_1_"
+  ], 

@pbauer this is due to your removal of the admin use in the p.a.contenttypes test fixture. I would suggest that we remove that user for all branches of p.a.contenttypes. This could potentially break test setups. Though, since we already released p.a.contenttypes with the admin-user removal there is no way back on this.

+  "customViewFields": [
+    "Title", 
+    "Creator", 
+    "Type", 
+    "ModificationDate"
   ], 
-  "customViewFields": [], 

Not sure why the customViewFields differ.

   "description": "This is an event", 
   "effective": null, 
-  "end": null, 
+  "end": "2013-01-01T12:00:00", 
   "event_url": null, 
   "exclude_from_nav": false, 
   "expires": null, 
@@ -27,7 +26,7 @@ Content-Type: application/json
   "language": "", 
   "location": null, 
   "modified": "2016-01-21T03:24:11+00:00", 
-  "open_end": null, 
+  "open_end": false, 
   "parent": {
     "@id": "http://localhost:55001/plone", 
     "@type": "Plone Site", 
@@ -42,11 +41,10 @@ Content-Type: application/json
     "@id": "http://localhost:55001/plone/event/@sharing", 
     "title": "Sharing"
   }, 
-  "start": null, 
+  "start": "2013-01-01T10:00:00", 
   "subjects": [], 
   "sync_uid": null, 
   "text": null, 
-  "timezone": null, 
   "title": "Event", 
-  "whole_day": null
+  "whole_day": false
 }

@thet it seems the implementation of plone.app.event has been changed considerably between Plone 5.0 and 5.1. Could you please elaborate the reasoning behind this?

+      "description": "This name will be displayed in the URL.", 
+      "title": "Short name", 
+      "type": "string"
+    }, 

It seems the default behavior of the id field has been changed in Plone 5.1. @plone/framework-team any idea?

   {
-    "@id": "http://localhost:55001/plone/@vocabularies/Group Ids", 
-    "title": "Group Ids"
+    "@id": "http://localhost:55001/plone/@vocabularies/plone.app.vocabularies.Users", 
+    "title": "plone.app.vocabularies.Users"
   }, 

Then we have tons of changes in the vocabularies. @plone/framework-team, any clues?

@jensens

This comment has been minimized.

Show comment
Hide comment
@jensens

jensens Mar 28, 2017

Member

How are the vocabularies fetched in plone.restapi? Group Ids is very different from plone.app.vocabularies.Users and should not be confused. Also, I think the Group Ids identifier is wrong in the first place. Plone vocabularies are registered by a name prefixed plone.app.vocabularies and should never contain a space.

Member

jensens commented Mar 28, 2017

How are the vocabularies fetched in plone.restapi? Group Ids is very different from plone.app.vocabularies.Users and should not be confused. Also, I think the Group Ids identifier is wrong in the first place. Plone vocabularies are registered by a name prefixed plone.app.vocabularies and should never contain a space.

@tisto

This comment has been minimized.

Show comment
Hide comment
@tisto

tisto Mar 29, 2017

Member

@jensens sorry, my example might have been misleading. My point is that multiple vocabularies do not match, see the travis link for the full example.

The vocabularies lookup:

https://github.com/plone/plone.restapi/blob/master/src/plone/restapi/services/vocabularies/get.py#L50

The vocabularies serialization happens here:

https://github.com/plone/plone.restapi/blob/master/src/plone/restapi/serializer/vocabularies.py#L14

Though, a blank space in an identifier does not seem right...

cc @csenger

Member

tisto commented Mar 29, 2017

@jensens sorry, my example might have been misleading. My point is that multiple vocabularies do not match, see the travis link for the full example.

The vocabularies lookup:

https://github.com/plone/plone.restapi/blob/master/src/plone/restapi/services/vocabularies/get.py#L50

The vocabularies serialization happens here:

https://github.com/plone/plone.restapi/blob/master/src/plone/restapi/serializer/vocabularies.py#L14

Though, a blank space in an identifier does not seem right...

cc @csenger

@jensens

This comment has been minimized.

Show comment
Hide comment
@jensens

jensens Mar 29, 2017

Member

I have double checked if registered names were changed in plone.app.vocabularies, but they are very stable. a git blame shows they are untouched for up to 10 years.
-> https://github.com/plone/plone.app.vocabularies/blame/master/plone/app/vocabularies/configure.zcml

So in fact I have no clue whats wrong there.

Member

jensens commented Mar 29, 2017

I have double checked if registered names were changed in plone.app.vocabularies, but they are very stable. a git blame shows they are untouched for up to 10 years.
-> https://github.com/plone/plone.app.vocabularies/blame/master/plone/app/vocabularies/configure.zcml

So in fact I have no clue whats wrong there.

@tisto

This comment has been minimized.

Show comment
Hide comment
@tisto

tisto Mar 29, 2017

Member

@jensens yeah. Seems so. I was hoping someone could give us a pointer where this might come from...

Member

tisto commented Mar 29, 2017

@jensens yeah. Seems so. I was hoping someone could give us a pointer where this might come from...

@tisto tisto added this to the 1.0a15 milestone Apr 18, 2017

@tisto tisto referenced this issue Apr 18, 2017

Closed

[WIP] Roadmap to plone.restapi 1.0 #274

11 of 19 tasks complete
@tisto

This comment has been minimized.

Show comment
Hide comment
@tisto

tisto Apr 25, 2017

Member

@pbauer did you had time to look into those issues during PLOG?

Member

tisto commented Apr 25, 2017

@pbauer did you had time to look into those issues during PLOG?

@pbauer

This comment has been minimized.

Show comment
Hide comment
@pbauer

pbauer Apr 25, 2017

Member

@tisto sorry, no. PLOG was a talk-only sprint for me.

Member

pbauer commented Apr 25, 2017

@tisto sorry, no. PLOG was a talk-only sprint for me.

@tisto

This comment has been minimized.

Show comment
Hide comment
@tisto

tisto Apr 25, 2017

Member

@pbauer ok, fixing those issues is kind of a pre-condition for writing PLIPs to include plone.restapi in core. If plone.restapi does not support Plone 5.1, a PLIP does not make much sense, right?

Member

tisto commented Apr 25, 2017

@pbauer ok, fixing those issues is kind of a pre-condition for writing PLIPs to include plone.restapi in core. If plone.restapi does not support Plone 5.1, a PLIP does not make much sense, right?

@gforcada

This comment has been minimized.

Show comment
Hide comment
@gforcada

gforcada Apr 26, 2017

Contributor

You can still write a PLIP and there decide if you want to make a version 2 for plone 5.1 and onwards (so you can get rid of legacy code and make the test setup easier) or if you plan to keep compatibility wit 4.3 and 5.0, which from a release/framework team point of view is where more effort would need to go

Contributor

gforcada commented Apr 26, 2017

You can still write a PLIP and there decide if you want to make a version 2 for plone 5.1 and onwards (so you can get rid of legacy code and make the test setup easier) or if you plan to keep compatibility wit 4.3 and 5.0, which from a release/framework team point of view is where more effort would need to go

@tisto

This comment has been minimized.

Show comment
Hide comment
@tisto

tisto May 7, 2017

Member

@gforcada I do not think writing a PLIP is the right response to a regression, neither is creating two versions of plone.restapi and put a lot of effort into backporting changes back and forth.

Our test layers used to be stable and consistent for Plone 4.x and 5.0. Due to inconsistent, undocumented and maybe even unconscious code changes in the Plone 5.1 releases of plone.app.contenttypes/plone.app.event/plone.app.testing/plone.testing (just guessing here, at least for the last two), we broke this stability. Do we really want hundreds of add-on developers to work around this instead of just fixing our core packages?

Just to be clear. This issue has nothing to do with plone.restapi. We are about to break our testing setup IMHO if we release Plone 5.1 like this. plone.restapi can not support Plone 5.1 if we don't fix this and a PLIP to include plone.restapi in core does not make any sense right now.

cc @plone/framework-team @esteele

Member

tisto commented May 7, 2017

@gforcada I do not think writing a PLIP is the right response to a regression, neither is creating two versions of plone.restapi and put a lot of effort into backporting changes back and forth.

Our test layers used to be stable and consistent for Plone 4.x and 5.0. Due to inconsistent, undocumented and maybe even unconscious code changes in the Plone 5.1 releases of plone.app.contenttypes/plone.app.event/plone.app.testing/plone.testing (just guessing here, at least for the last two), we broke this stability. Do we really want hundreds of add-on developers to work around this instead of just fixing our core packages?

Just to be clear. This issue has nothing to do with plone.restapi. We are about to break our testing setup IMHO if we release Plone 5.1 like this. plone.restapi can not support Plone 5.1 if we don't fix this and a PLIP to include plone.restapi in core does not make any sense right now.

cc @plone/framework-team @esteele

@tisto

This comment has been minimized.

Show comment
Hide comment
@tisto

tisto Jul 16, 2017

Member

Fixing Plone 5.1 test layers is beyond the scope of plone.restapi and it seems nobody cares about this anyways. plone.restapi works with Plone 5.1. We have to create issues in plone.app.contenttypes et at. Closing this issue...

Member

tisto commented Jul 16, 2017

Fixing Plone 5.1 test layers is beyond the scope of plone.restapi and it seems nobody cares about this anyways. plone.restapi works with Plone 5.1. We have to create issues in plone.app.contenttypes et at. Closing this issue...

@tisto tisto closed this Jul 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment