Conversation
|
@davisagli thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment: To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically. Happy hacking! |
|
Note: needs more investigation to see why the 2nd export isn't the same if we export, then import, then export |
|
@ericof The test failure is, at least in part, because the plone site is getting imported inside the existing plone site, instead of replacing it. Any ideas how we should handle this? |
…rn the parent of a Plone Site during functional tests
|
@davisagli I would suggest accepting an option to filter what do you want to be exported (i.e.: Not exporting portlets, or translations) |
ericof
left a comment
There was a problem hiding this comment.
I would like to have a way of selecting what is going to be exported, but other than that this is really good.
|
@ericof Can we merge it as is and then add more options as an improvement? We should make the new options consistent between the API and the CLI, and I don't want to tackle it today. |
|
Go for it |
mauritsvanrees
left a comment
There was a problem hiding this comment.
Nice! I may want to use this to export and import content from one instance of a Ploneconf website to another.
I will try it soon.
Meanwhile the code looks good. There is one typo (missing search-replace) in the tests, see inline suggestion.
mauritsvanrees
left a comment
There was a problem hiding this comment.
Seems to work fine when trying it locally on an almost bare Classic UI site.
Two more inline suggestions for the readme, to fix the curl commands.
|
One comment in general. I remember from a long time ago that you could make a Of course the technology underlying the rest api is quite different. But if in real world usage you see that the export takes much longer than expected, then think back to this remark. We don't need to do anything at this point, we will cross that bridge when we find it. |
Co-authored-by: Maurits van Rees <maurits@vanrees.org>
Co-authored-by: Maurits van Rees <maurits@vanrees.org>
|
@mauritsvanrees Thanks for the review! Yeah, I know this will not work well for exporting large sites. But we have the CLI script as well. The goal here is to give another option that might be more convenient for small exports. |
|
@davisagli Reviewing the docs today, I realised that probably this is the first core package where we create a RESTAPI endpoint outside |
|
@sneridagh This is a good point for us to discuss. I like keeping the service implementation alongside the other code it's closely related to, but we should try to have the documentation unified for everything that's in core (or core add-ons). It's too much work to maintain docs here and pull them into plone.restapi's docs automatically. I can work on adding the docs in plone.restapi. |
|
@davisagli Don't get me wrong, I agree that the implementation has to live where the code is. We have to make sure that the docs remain unified, maybe with the proper mention where the code is, if it makes sense. |
Fixes #87