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

Refs #136 - Update dynamic schema on all ZEO clients on change #137

Merged
merged 3 commits into from
Oct 8, 2020

Conversation

avoinea
Copy link
Sponsor Member

@avoinea avoinea commented Oct 2, 2020

Fixes #136

@mister-roboto
Copy link

@avoinea thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

@avoinea
Copy link
Sponsor Member Author

avoinea commented Oct 2, 2020

@jenkins-plone-org please run jobs

Copy link
Sponsor Member

@jensens jensens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple and elegant to use the _p_mtime as part of the generated schema name!

@jensens
Copy link
Sponsor Member

jensens commented Oct 3, 2020

@jenkins-plone-org please run jobs

1 similar comment
@jensens
Copy link
Sponsor Member

jensens commented Oct 5, 2020

@jenkins-plone-org please run jobs

@avoinea
Copy link
Sponsor Member Author

avoinea commented Oct 5, 2020

@jensens I'll take a look at the remaining failing tests...

@avoinea
Copy link
Sponsor Member Author

avoinea commented Oct 5, 2020

@jensens I don't know what is wrong with this one https://jenkins.plone.org/job/pull-request-5.2/1643/testReport/junit/(root)/(empty)/schemaeditor_txt/ and where it comes from.

For the rest, I added PRs within plone.app.dexterity and plone.app.contenttypes

plone/dexterity/schema.py Outdated Show resolved Hide resolved
Copy link
Member

@ale-rt ale-rt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks a lot!
Please, consider removing that log.info statement added by this PR or to change it with a log.debug.

plone/dexterity/schema.py Show resolved Hide resolved
@ale-rt
Copy link
Member

ale-rt commented Oct 5, 2020

@jenkins-plone-org please run jobs

1 similar comment
@ale-rt
Copy link
Member

ale-rt commented Oct 5, 2020

@jenkins-plone-org please run jobs

@ale-rt
Copy link
Member

ale-rt commented Oct 5, 2020

@avoinea The Python 3.7 test failure looks unrelated, but there is a real issue with https://jenkins.plone.org/job/pull-request-5.2/1652/testReport/(root)/(empty)/schemaeditor_txt/

Anyway we are close to have this merged :)

@avoinea
Copy link
Sponsor Member Author

avoinea commented Oct 7, 2020

@ale-rt Hmmm:

plone.app.relationfield-2.0.2-py2.7.egg/plone/app/relationfield/tests/../schemaeditor.txt

@avoinea
Copy link
Sponsor Member Author

avoinea commented Oct 8, 2020

@ale-rt
Copy link
Member

ale-rt commented Oct 8, 2020

@jenkins-plone-org please run jobs

@avoinea avoinea removed the request for review from tiberiuichim October 8, 2020 11:18
@avoinea
Copy link
Sponsor Member Author

avoinea commented Oct 8, 2020

@ale-rt @jensens How can we have this released also for Plone 4.3.x?

@ale-rt ale-rt merged commit 8fd6060 into master Oct 8, 2020
@ale-rt ale-rt deleted the sync-schema-on-zeo-clients branch October 8, 2020 12:46
@ale-rt
Copy link
Member

ale-rt commented Oct 8, 2020

According to https://github.com/plone/buildout.coredev/blob/a4af25bfde395a999844324b45b9912952a6731e/sources.cfg#L45 4.3 usese the plone.dexterity 2.2.x branch.
Theoretically you can backport the patch there before the 4.3.20 is release, see plone/Products.CMFPlone#3166.
I guess other packages will need fixes like the one involved in this PR.

Sincerely I would save myself from that pain.
Also personally I am not sure I will want to approve it.
In the past we had even simpler backports to 4.3 that caused major pain.

@mauritsvanrees what do you think about backporting this PR to 4.3?

@mauritsvanrees
Copy link
Sponsor Member

Plone 4.3.20 has already been released on 28 August. It has been waiting since then for a Universal Installer.

I don't mind doing a release from the plone.dexterity 2.2.x branch, but it will never get in an official release.

@mauritsvanrees
Copy link
Sponsor Member

BTW, the same is true for Plone 5.1.7: it was released a week ago, but waiting on installer. It is the last release from the Plone 5.1 series, so any updates to the relevant plone.dexterity branch, 2.6.x, would not get in an official release.
Again, an individual package release is still possible.

@tisto
Copy link
Sponsor Member

tisto commented Dec 17, 2020

@avoinea in plone.restapi we have tests that check for undocumented changes in our auto-generated API docs. This PR causes the auto-generated behavior names to change for every test run:

https://github.com/plone/plone.restapi/pull/1033/checks?check_run_id=1572135872

Therefore it is impossible for us to generate a stable REST API documentation. Is there a way to disable this behavior?

@avoinea
Copy link
Sponsor Member Author

avoinea commented Dec 18, 2020

@tisto True.

We have 2 options I can think of right now:

  • Either patch portalTypeToSchemaName before the tests,
  • or tweak the object _p_mtime

Another, elegant solution would be to convert portalTypeToSchemaName into an utility to be able to easily override it on need.

@tisto
Copy link
Sponsor Member

tisto commented Mar 22, 2021

@avoinea could you look into fixing this in plone.restapi maybe? I don't really have the time for this right now and I ran into this again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SchemaCache is not updated across multiple Plone instances
6 participants