-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Plone: Add info about Plone 6 Docker images #2479
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
base: master
Are you sure you want to change the base?
Conversation
I have to admit I'm a little confused -- does this mean the https://hub.docker.com/_/plone image is effectively deprecated and won't be getting 6.x+? 😅 (If so, we should make this change a little differently, especially with a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bah, I cannot request changes, only make a comment.
Anyway, versions are not features, so I created a new section, which should handle the concerns expressed.
I also added suggestions to update links to the old documentation for previous versions.
Co-authored-by: Steve Piercy <[email protected]>
Co-authored-by: Steve Piercy <[email protected]>
Apparently my suggestion to add a section did not get saved in my review. I'll try again, now that I have permission. |
@tianon As Plone 6 is not a monolith app anymore and it comes with a decoupled React front-end we don't have a consensus in the Plone Community on what we should do with this image, yet 🙈 I can see there are other apps that tries to put everything in a all-in-one-docker-image like https://github.com/nextcloud/all-in-one?tab=readme-ov-file#nextcloud-all-in-one but I'm not sure what is the best way to handle this situation for the Official Docker Images. Maybe you can point out to some good examples / practices.
Meanwhile, until we have a decision, I added the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As Plone Release Manager this change looks fine to me.
I don't have experience with how Docker Hub handles these files though, and what the recommendations are.
Quick background:
- For Plone 5 this image is probably fine, but Plone 5 only gets security support until October this year.
- For Plone 6 indeed
plone/plone-backend
andplone/plone-frontend
should be used and have been available since at least 2022. - I don't know if those two could somehow be promoted to official Docker images, or if it would be better to somehow combine them in one (which seems a bit tricky as two processes should run).
FYI, I'll be back from vacation tonight, and I'll look into how these docs are auto-generated from their counterparts in the Plone GitHub repos, and work on PRs there. From my quick glance of Docker's docs, it looks like the PRs will go under Plone first, then will get generated in Docker repos. |
Ah, interesting! I would definitely not recommend doing something like an all-in-one, because I have yet to see a truly reliable (and still minimal) "process supervisor" to help manage multiple processes in one container. Two images is perfectly reasonable, especially if the documentation describes how to run them (something like |
This is my review. In addition to the new section with versions, I overhauled the docs to comply with Plone 6 Documentation guidelines, mostly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Please merge!
Closes plone/plone.docker#179. |
I don't have merge permission. Who does? After this PR is merged, then I can start work on promoting plone-backend and plone-frontend as official Docker images. I'll merge the docs files from this PR with those from the other repos, then open a pull request for each image. When Plone 5.x exits security support, then I think we should remove just |
Ready for another run of workflows. |
I missed a couple of details, and I think I finally got them all this time. Ready for one more run. I didn't realize I could run the CI check locally with |
Just to make sure we all have the same understanding, the intention is to deprecate the |
@whalelines, I don't understand what you mean by two variants. @tianon said:
Are the docs in this PR technically approved? I've been waiting for that piece before I start. If approved, then I can start today on the frontend and backend images' docs in separate PRs. Please let me know. Thank you! |
There are currently three repositories serving Plone container images.
Currently, this PR for updating the documentation for the What @tianon suggested was that the I just want to make sure your intention is to deprecate the |
@whalelines thanks for the explanation and taking care. Can you point me to docs about how to do this, and an example project in the DOI repo? I'm not sure exactly what to search for. @mauritsvanrees @avoinea I'm available to chat in Discord to discuss how to move forward, and come up with a plan. This turned into something bigger than mere changes to docs that I expected. |
Currently the plone bashbrew file has a single stanza. You would just add a distinct stanza for each variant, e.g., aerospike, arangodb, backdrop, etc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems OK now, although perhaps you'd like to hold off on the deprecation notice now that you're planning to add plone:6
images to DOI? (docker-library/official-images#17281 (comment))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@avoinea please have a look.
Co-authored-by: Steve Piercy <[email protected]>
plone/deprecated.md
Outdated
- Plone 5.x is no longer supported. | ||
- It is strongly recommended to migrate to the latest available Plone version. See https://plone.org/download/release-schedule for details. | ||
|
||
The official Docker images for Plone 5.x and Plone 4.x are available here, and are maintained by community volunteers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be present tense, or past tense? (I don't think they're being actively maintained right now, are they?)
We should also add a DEPRECATED;
prefix to README-short.txt
(as seen in #2583).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To add a little more detail for #2583, my goal there was to make it really clear (immediately) to users on the page that any images they find here are old, outdated, and unsupported, and they should look elsewhere ASAP - I think this description gets there, but it's not quite as immediately obvious.
I don't have a strong preference either way, as long as we make it clear that this repository is officially deprecated and unsupported (and I think the tense here is the only blocking issue I have with that).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tianon I lack permission to make a comment on your PR #2583, so here it is:
plone/deprecated.md
is not listed as one of those in docker-library/docs
. Is it used or ignored? See https://github.com/docker-library/docs?tab=readme-ov-file#files-related-to-an-images-docs.
I'm revising this PR to incorporate your suggestions, once we verify that question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a few revisions. Hopefully this file gets included in the repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
…d to only v6+. ping @tianon.
After this PR is merged, we can proceed with the checklist at plone/plone.docker#179 (comment). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't love how many load-bearing content updates there are here (as you'll see below, my remaining actual review comments are all in content.md
), but it's mostly fine and I'm happy to see this close to getting over the line.
- Plone 5.x | ||
- Plone 4.x | ||
|
||
See the [deprecation notice](./deprecated.md) for Plone 6 and later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once this description ends up on Docker Hub (which is ultimately what all these files are for), this cross-file link won't work. I would suggest just wording this something like "See the deprecation notice above" or similar.
@@ -4,13 +4,19 @@ | |||
|
|||
%%LOGO%% | |||
|
|||
## Supported versions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wording is really tripping me up -- as I understand it, neither of these versions are "supported" either upstream or here, which is the whole point of adding the deprecation notice. Also, why is this duplicating the content from the deprecation notice which will appear on Docker Hub just above it?
|
||
```console | ||
-e ADDONS="eea.facetednavigation collective.easyform" \ | ||
-e VERSIONS="eea.facetednavigation=13.3 collective.easyform=2.1.0" | ||
``` | ||
|
||
RestAPI: | ||
To use Plone REST API, start the Plone Docker image, then issue a command to `curl`. | ||
|
||
```console |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're dropping the $
prefix on the command lines, this is no longer console
but rather shell
or bash
:
```console | |
```bash |
|
||
The official Docker images for Plone 5.x and Plone 4.x are available here. | ||
|
||
Beginning with Plone 6.x, the frontend and backend run in separate Docker images. There's also a a ZEO server Docker image. All Plone 6.0 and later images are supported by the Plone Foundation and are maintained by community volunteers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't love that it's not completely clear that this repository (https://hub.docker.com/_/plone) is completely deprecated, but I can live with this wording.
No description provided.