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

Dependency fix #45

Merged
merged 16 commits into from
May 3, 2023
Merged

Dependency fix #45

merged 16 commits into from
May 3, 2023

Conversation

petschki
Copy link
Member

@petschki petschki commented Apr 27, 2023

Dependency on ZEO and zdaemon are lost when defining additional eggs in the recipe. Make sure we do not loose our required dependencies in that case.

see discussion https://community.plone.org/t/plone-6-0-4-released/17406/7?u=petschki

@mister-roboto
Copy link

@petschki thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

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!

@petschki petschki requested a review from mauritsvanrees April 27, 2023 12:25
@petschki
Copy link
Member Author

@jenkins-plone-org please run jobs

@petschki petschki requested a review from jensens April 27, 2023 12:36
@petschki
Copy link
Member Author

I think this issue gets fixed with this too #29

@petschki
Copy link
Member Author

pre-commit.ci autofix

@petschki
Copy link
Member Author

@jenkins-plone-org please run jobs

@petschki petschki changed the title Petschki dependency fix Dependency fix Apr 27, 2023
Copy link
Member

@mauritsvanrees mauritsvanrees left a comment

Choose a reason for hiding this comment

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

Mostly good, thanks! One inline comment with a small request for Windows, but that is really unrelated to this PR.

But with the plone/meta changes we also run pyupgrade to convert the code to Python 3.8+ only. And latest black is run. So this will break Python 2 compatibility. So this is a breaking change.

I am okay with that. But it means the setup.py can be cleaned up a bit more, changelog updated. maybe the readme.

I have created a 2.x branch from current master, and have let coredev 5.2 use this.

setup.py Outdated Show resolved Hide resolved
@mauritsvanrees
Copy link
Member

BTW, I don't think this has any influence on Jenkins, so those jobs do not need to run. We should let mr.roboto ignore this repo.

@mauritsvanrees
Copy link
Member

BTW, I don't think this has any influence on Jenkins, so those jobs do not need to run. We should let mr.roboto ignore this repo.

Okay, I added this in a mr.roboto PR that I already created earlier today for something else.

@petschki petschki force-pushed the petschki-dependency-fix branch from 41ca172 to 4e3197f Compare April 28, 2023 06:04
@petschki
Copy link
Member Author

@jenkins-plone-org please run jobs

@petschki petschki requested a review from mauritsvanrees April 28, 2023 06:05
@petschki
Copy link
Member Author

Can anyone activate GHA here?

@petschki petschki force-pushed the petschki-dependency-fix branch from 85ac26d to 8e950b6 Compare April 28, 2023 06:19
@petschki petschki force-pushed the petschki-dependency-fix branch from e1608b6 to bde1615 Compare April 28, 2023 07:15
@mauritsvanrees
Copy link
Member

Can anyone activate GHA here?

The central workflow? Done.

@petschki petschki force-pushed the petschki-dependency-fix branch from 2a9bc00 to d007ad0 Compare April 28, 2023 08:05
@petschki petschki force-pushed the petschki-dependency-fix branch from d007ad0 to f4df9be Compare April 28, 2023 08:07
@petschki
Copy link
Member Author

@jenkins-plone-org please run jobs

@petschki petschki force-pushed the petschki-dependency-fix branch from f920b2c to 8dc0b1f Compare May 2, 2023 11:51
@petschki petschki force-pushed the petschki-dependency-fix branch from 8dc0b1f to 12b1c23 Compare May 2, 2023 11:54
@petschki
Copy link
Member Author

petschki commented May 2, 2023

@jenkins-plone-org please run jobs

@petschki
Copy link
Member Author

petschki commented May 2, 2023

I testet this on my Parallels Win11 and it worked. Tests are also fixed. @mauritsvanrees what do you think?

@petschki
Copy link
Member Author

petschki commented May 2, 2023

@icemac we are pinning zope.mkzeoinstance=5.0 for Plone 6.0 because on 5.1 there's a traceback:

Traceback (most recent call last):
  File "/Users/peterm/.pyenv/versions/3.11.3/envs/py-3.11-familienpass/lib/python3.11/site-packages/zc/buildout/buildout.py", line 2252, in main
    getattr(buildout, command)(args)
  File "/Users/peterm/.pyenv/versions/3.11.3/envs/py-3.11-familienpass/lib/python3.11/site-packages/zc/buildout/buildout.py", line 856, in install
    installed_files = self[part]._call(recipe.install)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/peterm/.pyenv/versions/3.11.3/envs/py-3.11-familienpass/lib/python3.11/site-packages/zc/buildout/buildout.py", line 1652, in _call
    return f()
           ^^^
  File "/Users/peterm/workspace/familienpass/src/plone.recipe.zeoserver/src/plone/recipe/zeoserver/recipe.py", line 88, in install
    ZEOInstanceBuilder().create(location, params)
  File "/Users/peterm/workspace/familienpass/eggs/zope.mkzeoinstance-5.1-py3.11.egg/zope/mkzeoinstance/__init__.py", line 188, in create
    makefile(ZEO_CONF_TEMPLATE, home, "etc", "zeo.conf", **params)
  File "/Users/peterm/workspace/familienpass/eggs/zope.mkzeoinstance-5.1-py3.11.egg/zope/mkzeoinstance/__init__.py", line 256, in makefile
    data = template % kwds
           ~~~~~~~~~^~~~~~
KeyError: 'blob_dir'

do we have to take care about that in this recipe, or is this a bug in zope.mkzeoinstance ?

@icemac
Copy link

icemac commented May 3, 2023

@petschki The problem with zope.mkzeoinstance was probably introduced by zopefoundation/zope.mkzeoinstance#16 which added support for blob_dir, so it should be fixed over there. (A PR is welcome.)

@mauritsvanrees
Copy link
Member

I testet this on my Parallels Win11 and it worked. Tests are also fixed. @mauritsvanrees what do you think?

Looking good now. Thanks!

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.

5 participants