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

Import loader from YAFOWIL. #98

Merged
merged 3 commits into from
Feb 21, 2020
Merged

Conversation

al45tair
Copy link
Contributor

Without doing this, various built-in widgets aren't registered, which
results in mysterious KeyError messages when people try to configure
LDAP in Plone.

Fixes #97 and #92.

Without doing this, various built-in widgets aren't registered, which
results in mysterious KeyError messages when people try to configure
LDAP in Plone.

Fixes collective#97 and collective#92.
@al45tair al45tair requested review from reinhardt and jensens and removed request for reinhardt February 20, 2020 18:22
Copy link
Contributor

@reinhardt reinhardt left a comment

Choose a reason for hiding this comment

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

Fixing things with a magic import makes me suspicious in general, but finding a cleaner solution might not be worth the effort. So I'm OK with it for the moment.

@al45tair
Copy link
Contributor Author

@reinhardt FWIW, the "magic import" is what the YAFOWIL documentation tells you to do. My guess (I haven't checked) is that what happened that broke it is that someone set things up to check everything with Flake8, which said it wasn't used, so it got deleted. That might not have broken on some peoples' setups because they may be doing that import in some other module, so YAFOWIL ends up with everything registered properly because it's already pulled in elsewhere.

I'm not a huge fan of the magic import either, but IMO that's really something that would need addressing in YAFOWIL rather than here.

@reinhardt
Copy link
Contributor

I'm not a huge fan of the magic import either, but IMO that's really something that would need addressing in YAFOWIL rather than here.

I agree. If @jensens has no further reservations I would merge it like this.

@rnixx
Copy link
Contributor

rnixx commented Feb 21, 2020

the import of yafowil.loader actually registers all blueprints of yafowil and addon widgets to the yafowil factory. Maybe we change this in future and provide a dedicated function call to avoid confusion. Though, as this would be an API break, it probably will not happen very soon.

@rnixx rnixx merged commit ae8ee7e into collective:master Feb 21, 2020
@rnixx
Copy link
Contributor

rnixx commented Feb 21, 2020

someone should drop a release. @jensens?

@reinhardt
Copy link
Contributor

I can do that. I'll make it 1.7.2.

@reinhardt
Copy link
Contributor

... except that my release setup seems to be broken in some way. I'd be grateful if someone else could take care of it after all.

@reinhardt
Copy link
Contributor

Ah! Now it worked. Strange. But that should be all done now.

@NicolasGoeddel
Copy link

I just installed pas.plugins.ldap on Plone 5.2.1 (5208) and I also got the KeyError: 'form' thing here.
Buildout tells me that it has installed version 1.7.2. After reading this issue I thought this would be enough to solve the error. But it seeems not. How can I fix the issue then? Also the Plone extension manager tells me that pas.plugins.ldap should not be uninstalled. How to upgrade then? I am just curious.

@cekk
Copy link

cekk commented Apr 27, 2020

same problem here: fresh install of pas.plugins.ldap (1.7.2) and i have the same error of @NicolasGoeddel

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.

1.7.1 error with plone 5.2.1
5 participants