Checkout Products.CMFPlone to have a green GHA build again

[ale-rt]

This fixes the build like it is defined now, but I think that the GHA workflows should be revisited.

Fixes #1254

[mister-roboto]

@ale-rt 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!

[mauritsvanrees]

This branch is tested in 5.2 as well. We do not want to lose this.
Can we add the checkouts with mr.developer?

[ale-rt]

This branch is tested in 5.2 as well. We do not want to lose this. Can we add the checkouts with mr.developer?

Yeah, I will do that!

[ale-rt]

I actually came to a conclusion that the tests should just run for Plone 5.2.
There is also the funny thing that the tests rewrite some files and this script checks that the changes are all committed:
https://github.com/plone/plone.restapi/blob/master/test-no-uncommitted-doc-changes.in
But the script runs only on Plone 5.2/Python3.7.

So we have now a package where we have a test that checks that the documentation in a Plone 6 only package is accurate for Plone 5.2/Py 3.7.

[ale-rt]

The build is green again!
I would avoid running the jenkins tests and I think a changelog is not needed.
@jensens or @mauritsvanrees, do you have the power to merge this?

[jensens]

This is in some way needed, but we should really think if it is a good idea to test high level module like this on GHA.

[tisto]

@mauritsvanrees @ale-rt @jensens can we please have a CMFPlone release to get rid of the checkout soonish? Checkouts of a different repo in a CI job is an anti-pattern, that we already got way too used to in the Plone community.

I understand that we have a chicken-egg problem here. Though, to fully get over this we need to cut a release. I never did a CMFPlone release. @mauritsvanrees, can you please take care of this?

There is no alternative to testing on GHA IMHO. plone.restapi is part of the Plone 6 frontend stack and needs to be loosely coupled to the backend. Therefore it needs its own CI infrastructure anyways. We barely manage to maintain our own Jenkins infrastructure and the new Docker infrastructure will also be based on GHA. Therefore I do not see a real alternative here. Maybe I missed your point though.

I hope that in the future this won't happen that often. They key to avoiding this is a stable "Plone core" and a loose coupling between the frontend stack and the backend stack. This will take a while though...

One more thing. I'd love to see a towncrier category "internal" for those kinds of things.

[ale-rt]

@mauritsvanrees @ale-rt @jensens can we please have a CMFPlone release to get rid of the checkout soonish? Checkouts of a different repo in a CI job is an anti-pattern, that we already got way too used to in the Plone community.

Our reference CI server is Jenkins, not GHA.
As long as you are using GHA you will have this kind of problems.
It is still fine and welcomed to have the linting tests in GHA.
But the kind of issues that this PR fixes can surface at any time.
In addition they are not adding anything at the moment because the tests about the documentation being up-to-date are only running for Plone 5.2 Python 3.7

I understand that we have a chicken-egg problem here. Though, to fully get over this we need to cut a release. I never did a CMFPlone release. @mauritsvanrees, can you please take care of this?

There is no alternative to testing on GHA IMHO. plone.restapi is part of the Plone 6 frontend stack and needs to be loosely coupled to the backend. Therefore it needs its own CI infrastructure anyways. We barely manage to maintain our own Jenkins infrastructure and the new Docker infrastructure will also be based on GHA. Therefore I do not see a real alternative here. Maybe I missed your point though.

I do not understand this point at all. Testing plone.restapi on jenkins is madatory. Testing them on GHA is not and it has been proved that the current GHA setup is brittle.

I will make a PR to fix this.

I hope that in the future this won't happen that often. They key to avoiding this is a stable "Plone core" and a loose coupling between the frontend stack and the backend stack. This will take a while though...

I still do not get it :)

One more thing. I'd love to see a towncrier category "internal" for those kinds of things.

It might be a nice addition, maybe people will like "chore" more, But I do think the name is super important :)

[tisto]

@ale-rt I will continue our discussion here and not in the new PR. :)

plone.restapi, plone.rest, plone.volto and Volto itself are the new frontend stack for Plone 6.

We have a pretty tight coupling between these components. When we change something in one component it is highly likely that we have to change something in another one as well. This is a lesson that we learned in the past few years.

The frontend stack for Plone 6 will continue to move faster than the backend. The only way to accomplish this is a loose coupling between the frontend stack and the backend stack.

The companies involved in the development of Volto are currently all on Plone 5.2, we will have to provide a smooth transition to Plone 6. Though, we do not have any plans to drop Plone 5.2 support in any of those packages (plone.restapi, plone.rest, plone.volto, Volto) anytime soon. That might happen when Plone 6 is stable. Though, so far this is not on the horizon.

plone.restapi, plone.rest and plone.volto are not regular Plone core packages, they are maintained by me, and they have to support multiple Plone versions at the same time.

Therefore they need their own CI system, no matter what.

Our Jenkins configuration is not in the best shape TBH and running all coredev tests for our packages is slow and the acceptance tests are super brittle.

I spend years of my open-source career fixing this. Though, we moved on with the new frontend stack to using GHA (and Cypress to run our tests). GHA is a lot faster, more stable, less maintenance work, more convenient and if I am not mistaken @ericof will not use Jenkins to build container images for Plone 6.

GHA is a reality for the new frontend stack that won't go away. I am not saying we should drop Jenkins, we put way too much effort into our setup and it works fine for Plone core and Classic. Though, for the new frontend stack we have to move on, unless anyone steps up and moves everything we have to Jenkins. :)

That being said, it does not make much sense IMHO to stop testing Plone 6.0 on GHA. The Plone 6 test on GHA gives us fast results and feedback. When the GHA tests are green, we ask the Plone Jenkins to run the CI job to make sure all tests fail (we still have to do this manually in 2021, not blaming anyone here but compared to GHA and a modern Jenkins setup that's connected to GH properly and runs those things automatically this feels pretty inconvenient).

I think we are pretty ok with our current setup and we have to live with edge cases like this one.

I hope I could explain our reasoning for the current setup. If not, I am happy to discuss this in a call on Discord. :)

[ale-rt]

I am now on the train and so I am not available for a call, sorry. Maybe tomorrow.

I am pretty aware of all the things you said before and I am not going to discuss things unrelated to the point of this PR and of #1259 but I will stick to the topic of having proper GHA actions.

I agree that GHA tests are faster, of course.
But they are not authoritative at all also because they are testing this package against an already old version of Plone 6, see:

• plone.restapi/plone-6.0.x.cfg

  Line 4 in 52b2bcf

  https://dist.plone.org/release/6.0-dev/versions.cfg

but with the checkout of Products.CMFPlone which is totally wrong IMO.
From my point of view this can cause much more headaches than the ones you want to avoid.

Jenkins, which BTW is doing an hell of a good job also because of the effort you put in it, is testing this package on Plone 6 only.
The classifiers in setup.py state that master is compatible with Plone 5.2.
I am totally fine with that and this is kind of proved by the GHA tests.

So I still think you should like and approve #1259 which purpose is to avoid complications.

[tisto]

@ale-rt the new Plone frontend stack needs to work on a wide variety of Plone core versions. People are using Plone 4,5 and 6 with restapi today to run Volto on top of it. This flexibility allows us to support Plone 6 in the time frames that Plone "the Product" needs.

plone.restapi works with multiple Plone versions and abstracts away the underlying differences of the backend. To do so, plone.restapi needs to be loosely coupled from the backend, in code, in tests and also in CI. This is why the release and the FWT acknowledged the "special" status of plone.restapi and plone.rest and why those packages have their own release cycle and release manager.

I understand your point of view and if you take into account Plone Classic/Core into account you are correct. Though, you have to understand that we have to take into account lots of different perspectives and use cases here.

Let's try to schedule a call tomorrow. I have time in the afternoon.

[mauritsvanrees]

For me, given the needs of Volto, it is fine to keep testing plone.restapi master on GHA on 5.2 and 6.0, next to Jenkins on 6.0.

Technically, since plone.restapi is only loosely coupled to the backend, you could ask: "Why don't the companies who use latest Volto and restapi simply use Plone 6? plone.restapi smoothes over the differences, right?" But Plone 6 is still in alpha, so I can understand reluctance to live both on the frontend edge and the backend edge.

It may have been nice, if the latest changes would have been done in such a way that the plone.restapi tests work both with CMFPlone 6.0.0a1 (on GHA) and with CMFPlone checkout (on Jenkins). But that effort would have been in vain after a second CMFPlone alpha release. So for me, the current solution with checking out CMFPlone seems the best way.

I can understand the wish for this checkout to be temporary. But is it really a problem if the checkout stays for a few weeks? Technically I could release a second CMFPlone alpha, but there are only tiny changes in the whole of Plone compared to the first alpha. To me it does not make much sense to release a second alpha so soon.

Other points:

• GHA tests against https://dist.plone.org/release/6.0-dev/versions.cfg. I think this is exactly what we want. This currently has the same versions as 6.0.0a1, but I will keep the 6.0-dev updated (irregularly for now) with new versions.
• BTW, for 5.2 testing you could choose to use 5.2-latest (currently pointing to 5.2.6) or 5.2-dev (currently same versions as 5.2.6, but I can add new versions anytime).
• Internal category for towncrier: sure, you can add that. Or if you don't want a news snippet for this to end up in CHANGES.rst, simply do not add one. One of the mr.roboto checks will fail, but you can just add a comment for reviewers, saying that you think no changelog entry is needed. To add a category, edit pyproject.toml and add this:

[[tool.towncrier.type]]
directory = "internal"
name = "Internal:"
showcontent = true

[ale-rt]

@ale-rt the new Plone frontend stack needs to work on a wide variety of Plone core versions. People are using Plone 4,5 and 6 with restapi today to run Volto on top of it. This flexibility allows us to support Plone 6 in the time frames that Plone "the Product" needs.

I know that, but this is not related at all to the topic of this PR and #1259.

plone.restapi works with multiple Plone versions and abstracts away the underlying differences of the backend. To do so, plone.restapi needs to be loosely coupled from the backend, in code, in tests and also in CI. This is why the release and the FWT acknowledged the "special" status of plone.restapi and plone.rest and why those packages have their own release cycle and release manager.

I know that, but this is not related at all to the topic of this PR and #1259.

I understand your point of view and if you take into account Plone Classic/Core into account you are correct.

This is not related to Plone classic at all. Why do you suspect that?

Though, you have to understand that we have to take into account lots of different perspectives and use cases here.

I understand that but this is not related at all to the topic of this PR and #1259.

Let's try to schedule a call tomorrow. I have time in the afternoon.

I would like to have this in a written form. I fear that in a call it will be really hard to keep the discussion on topic and that will be running in circles.
I am just focusing on a technical point here and in #1259.

[ale-rt]

For me, given the needs of Volto, it is fine to keep testing plone.restapi master on GHA on 5.2 and 6.0, next to Jenkins on 6.0.

I actually would not mind if #1259 is rejected, my point was just to improve developers life and avoid the temporary problems with GHA we had before this PR was merged.

IMO (from a technical point of view) #1259 is the way to go because it removes tests that declare the package fit to run on top of a distribution of eggs nobody is (and should not) use in production.
But if you like to continue like it is it is not a major problem for me.
I rarely work on this package.

[tisto]

I would like to have this in a written form. I fear that in a call it will be really hard to keep the discussion on topic and that will be running in circles.

The call was just meant to figure this out between the two of us to make sure we are cool. It seems there are a few misunderstandings and we mutually do not fully get our arguments.

What I usually do when doing 1:1 calls is to add a note with the outcome of the call to make it transparent. If you prefer to continue this discussion here, I am ok with that. We can postpone this to the next time we meet in person for a beer. :)

For me, given the needs of Volto, it is fine to keep testing plone.restapi master on GHA on 5.2 and 6.0, next to Jenkins on 6.0.

I actually would not mind if #1259 is rejected, my point was just to improve developers life and avoid the temporary problems with GHA we had before this PR was merged.

I actually think that running into issues like we did is a feature of our current setup. If we want a loose coupling between frontend/backend, those things should be hard and painful. To make us aware that we are crossing lines that in an ideal world would not cause issues. If this causes major issues, we have to re-visit our decision of course.

IMO (from a technical point of view) #1259 is the way to go because it removes tests that declare the package fit to run on top of a distribution of eggs nobody is (and should not) use in production. But if you like to continue like it is it is not a major problem for me. I rarely work on this package.

This is a new argument that we did not discuss so far. We did not declare that plone.restapi works on Plone 6. I just added the Plone 6 job to do an early test of the first releases of Plone 6. I would actually like to continue to have this as additional feedback for our CI job. If it would cause massive problems, we have to re-visit.

[tisto]

Technically, since plone.restapi is only loosely coupled to the backend, you could ask: "Why don't the companies who use latest Volto and restapi simply use Plone 6? plone.restapi smoothes over the differences, right?" But Plone 6 is still in alpha, so I can understand reluctance to live both on the frontend edge and the backend edge.

This would indeed be nice. The transition to Plone 6 should be trivial. Though, as you wrote, Volto is still moving fast. So most of us currently prefer to put our "innovation points" in the frontend. This will change once we are out of alpha I guess.

I can understand the wish for this checkout to be temporary. But is it really a problem if the checkout stays for a few weeks? Technically I could release a second CMFPlone alpha, but there are only tiny changes in the whole of Plone compared to the first alpha. To me it does not make much sense to release a second alpha so soon.

Whatever works for you. I just expressed a wish here. You are the release manager and you make those decisions.

Other points:

• GHA tests against https://dist.plone.org/release/6.0-dev/versions.cfg. I think this is exactly what we want. This currently has the same versions as 6.0.0a1, but I will keep the 6.0-dev updated (irregularly for now) with new versions.
• BTW, for 5.2 testing you could choose to use 5.2-latest (currently pointing to 5.2.6) or 5.2-dev (currently same versions as 5.2.6, but I can add new versions anytime).

I personally strongly prefer to not rely on resources that are subject to change in a CI setup. Those kinds of external changes are the number one cause of CI problems that are very time-consuming to fix. For me, relying on any resource that is subject to change in a CI setup (like "-latest") is one of the worst anti-patterns for CI.

The additional time it takes to manually update the CI setup (and then see the problem that is caused by this upgrade) is very well invested time IMO.

• Internal category for towncrier: sure, you can add that. Or if you don't want a news snippet for this to end up in CHANGES.rst, simply do not add one. One of the mr.roboto checks will fail, but you can just add a comment for reviewers, saying that you think no changelog entry is needed. To add a category, edit pyproject.toml and add this:

[[tool.towncrier.type]]
directory = "internal"
name = "Internal:"
showcontent = true

Thanks for the hint! Will add this! Though, it would be good to create a standard for all Plone core packages that we all can follow.

[mauritsvanrees]

Thanks for the hint! Will add this! Though, it would be good to create a standard for all Plone core packages that we all can follow.

The standard is defined in plone.releaser and I think we (Gil and me mostly) made sure we have this in all Plone packages.

[ale-rt]

The call was just meant to figure this out between the two of us to make sure we are cool.

We are cool, do not even start thinking about the opposite!

It seems there are a few misunderstandings and we mutually do not fully get our arguments.

That's for sure :)

What I usually do when doing 1:1 calls is to add a note with the outcome of the call to make it transparent. If you prefer to continue this discussion here, I am ok with that. We can postpone this to the next time we meet in person for a beer. :)

That's always welcomed ;)

I actually think that running into issues like we did is a feature of our current setup. If we want a loose coupling between frontend/backend, those things should be hard and painful. To make us aware that we are crossing lines that in an ideal world would not cause issues. If this causes major issues, we have to re-visit our decision of course.

Ok, I respect that.

IMO (from a technical point of view) #1259 is the way to go because it removes tests that declare the package fit to run on top of a distribution of eggs nobody is (and should not) use in production. But if you like to continue like it is it is not a major problem for me. I rarely work on this package.

This is a new argument that we did not discuss so far. We did not declare that plone.restapi works on Plone 6. I just added the Plone 6 job to do an early test of the first releases of Plone 6. I would actually like to continue to have this as additional feedback for our CI job. If it would cause massive problems, we have to re-visit.

I think that what you call "a new argument" was my point since the beginning.
Anyway I will now close the PR #1259.
Too much blood already wasted for that one.