Use a ServiceLoader to discover WebEnvironments #53
Conversation
Signed-off-by: Brian Demers <bdemers@apache.org>
bdemers
force-pushed
the
environment-service
branch
from
2dfd7f4
to
a993801
Compare
|
@leleuj added a couple tests and fixed a couple minor bugs. It should be ready to go, but I still want input from buiji before merging |
bdemers
changed the title
|
It looks good to me, yet the real implementation in buji-pac4j will be necessary to confirm everything is ok. I plan to release buji-pac4j v2.0.4 next week, but then I'll be able to handle that. |
|
Great! keep me posted! |
|
On my way to release buji-pac4j, I just saw that the latest Shiro version is 1.4.0-RC2. Is there any plan/date for a GA release? |
|
Soon, I'd like to get this in first, and I there is another feature PR that
we migt be able to squeeze in. But no blockers.
…On Mon, Jan 16, 2017 at 4:13 AM LELEU Jérôme ***@***.***> wrote:
On my way to release buji-pac4j, I just saw that the latest Shiro version
is 1.4.0-RC2. Is there any plan/date for a GA release?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#53 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAGGcllu5EJ5oojIgbA6AsB_fp_Z_zt9ks5rSzSygaJpZM4LPgxJ>
.
|
|
OK. So I'm releasing a version 2.1.0-RC1 first (to accomodate wih 1.4.0-RC2). |
|
@leleuj bump Sorry to bug you, is there anything I can do to help move this along? |
|
No problem, I'm done with pac4j version 2.0.0-RC1. I will work on that (maybe without upgrading to pac4j v2 though). I'll keep you posted. |
|
Fairly straightforward! Is this what you had in mind: bujiio/buji-pac4j#54 + pac4j/buji-pac4j-demo#12? Thanks |
|
@leleuj defiantly the first step. I'm not sure this works for that demo project but, hacked up the PR's you mentioned to give an idea what I'm talking about (these both depend on this PR): First, define the service def: The use it automatically: I also swapped out the web.xml config for shiro's servlet fragment, but that is optional, just trying to demonstrate removing all of the basic config. This should allow any framework to inject itself into the Shiro's configuration by just including a dependency. Making it easier to get started with Shiro through another framework: Buji, Stormpath, Apache Zeppelin, Apache Geode, etc. Thoughts? |
|
OK. I see. But for sure, I tested it and it was working. But it's a lot better indeed to have this automatically detected. Is there some documentation somewhere? I couldn't find it. Will update and test it again. |
|
I updated the PR and tested your suggestions but it doesn't work, although it looks good. I tested everything with a local built Shiro and your PR merged in. Am I missing something? |
|
I'm not sure, the only thing that _should_ be needed is the META-INF
service definition. Try without the shiro-servlet-plugin, and just comment
out your web.xml init param for your Environment class. I probably
shouldn't have mixed these two topics together, I was just trying to show
the _ultimate goal_ of zero config. Sounds like we should keep them
separate until we test them with Buji both in isolation.
…On Wed, Feb 15, 2017 at 3:59 AM, LELEU Jérôme ***@***.***> wrote:
I updated the PR and tested your suggestions but it doesn't work, although
it looks good. I tested everything with a local built Shiro and your PR
merged in.
Am I missing something?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#53 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAGGcjr5labgVoQ4u_PZE9NjJ8O1Oy7Xks5rcr5WgaJpZM4LPgxJ>
.
|
|
OK. It's clearer now and the I merged both PR: the new version buji-pac4j 2.2 is ready. I guess we said we would wait for the final 1.4.0 release of Shiro before releasing again buji-pac4j: what's the next step? |
|
Great! I'll clean this up a bit, check test coverage, javadoc etc, and merge it. |
|
Coming back on this. I don't see any recent snapshot in the Apache repository. Can you merge this pull request? |
|
Yes! sorry I've been a bit slammed the past couple weeks, I'll get that in
today
…On Wed, Mar 15, 2017 at 3:04 AM, LELEU Jérôme ***@***.***> wrote:
Coming back on this. I don't see any recent snapshot in the Apache
repository. Can you merge this pull request?
When do you plan the 1.4.0 release? Thanks
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#53 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAGGchZRKosytzjOmzq44N8u-Pu45rwzks5rl42XgaJpZM4LPgxJ>
.
|
|
Thanks. Keep me posted for the release as well. |
|
@bdemers Can we merge that pull request? I'm flooded by complaints on the buji-pac4j-demo as it doesn't work, waiting for this PR. Thanks. |
|
@leleuj sorry about the wait, please feel free to yell at me if this happens again (or the wait is more then a weekend) I got caught up in some travel and forgot about this one. I just pushed to the ASF, so it should sync here any minute |
|
Great! I hate to yell a people doing open source for free ;-) |
|
Ha, agreed, but I did said I would do this a while ago, so you would be will in your right to yell :D Take a look through the open PR's, and see if there is anything that should be merged before we release, I know at least #62 should be, I can look at that tonight or tomorrow night. (and lets move this portion of the conversation to the dev list) |
|
I see 25 opened PRs: are they really? Because this one is still opened as well. I don't see anything special, except: #25 I guess we already merged it. |
|
The github/ASF sync occasionally leaves issues PRs open if they are not tagged correctly. We also don't have the ability to add labels to the PRs. A while back I went through the JIRA's and converted patches into Pull Requests, but that probably adds confusion. I'll go through them and mark the closed with a noticed to comment to reopen (they will still be linked to the original JIRA, so should still be valuable if anyone wants to pick them up) That will leave a smaller handful for consideration. If you see anything that you feel is more important then others please add comment in that issue. (note: the ASF infra is working on support these Github issues, so it should not always be like this) |
|
Given we already have a RC2, I would not add anything more for the final release. |
|
Reminder: @bdemers the final release is expected very soon... |
|
Already merged |
|
Is this the right process to follow: https://cwiki.apache.org/confluence/display/SHIRO/Performing+a+Release? I have a bunch of pac4j releases to perform this week, so I can handle it also in my "release week". |
|
👍 |
The idea here is to lessen the touch points for frameworks when integrating with Shiro. A property file and a
WebEnvironmentimplementation should be all that is needed. The WebEnvironment can then provide additional defaults or customizations specific to that framework.https://issues.apache.org/jira/browse/SHIRO-608