-
-
Notifications
You must be signed in to change notification settings - Fork 55
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
feat: Upgrade JHipster, Quarkus dependencies, fix broken flows #247
Conversation
@mraible, there seems an issue with embedded mongodb dependency and driver version. This is independent of current upgrade and can be tracked separately ( I believe there exists a ticket for mongodb migration). |
@mraible, did you get a chance to review the changes? I don't have commit access to this repository to assign reviewers. I am amidst another change to bump quarkus ecosystem dependencies and noticed a few more broken flows. I will raise separate PRs to fix those. |
@vishal423 it seems the way GitHub allows us to manage groups is a bit complicated: as administrator I can create a team but I can't add people to existing ones if I'm not the maintainer of this team... That doesn't make sense... So I've added you directly with the writing access. Let's see after how to fix that. |
@vishal423 I'm traveling and attending conferences this week. I'll take a look when I'm back home next week. |
@@ -13,5 +13,4 @@ insert_final_newline = true | |||
trim_trailing_whitespace = false | |||
|
|||
[{package,bower}.json] | |||
indent_style = space |
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.
Does this mean that indents are now tabs, or indents are spaces by default?
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.
It was redundant as we define to use spaces for all type of files:
[*]
indent_style = space
I tried this branch today with @danielpetisme's tutorial from the Okta developer blog. The initial console output is a bit lengthy, especially since there are two logos displayed. Also, there are a lot of "Using blueprint" messages. At the end of the application creating part, there's a printout of commands:
It looks like there should be a space between I was using Java 17 and received an error:
Reverting to Java 11 fixes this. However, I think we should support Java 17 since the main generator does. Finally, after starting Keycloak and running
Below is my
|
Hi @mraible |
The first logo is coming from the JHipster generator itself and I don't see an option to customize/hide that. If you refer to any other blueprint using the latest published JHipster version, it should be the same behavior. The
It also doesn't look correct to me. Either space or slash should come. I will take a look into that. However, files removal is the JHipster generator behavior and isn't controlled over here 😞
This PR doesn't touch on the Quarkus lib upgrade. I have a separate unpublished branch that contains the fix to support Java 17, and Quarkus lib upgrades for Maven, however, the OIDC flow seems to be broken. I didn't spend much time debugging the issue, however, that seems to come due to quarkus integration with the problem library. Unfortunately, I am a bit occupied and wouldn't get much time before mid-November. If you want to combine those changes, I can push them over here. |
OK. I'll take a look and see if I can figure out the problem with MongoDB.
Yes, please push your changes into another PR. If it's easier, you can merge this one first. |
I will push those lib changes over here, so that you can take a consolidated view of change. I haven't looked into mongo db flow and the dependencies upgrade. Additionally, I have replaced the in-memory Redis with devservices support (uses TestContainer), so, docker is going to be a requirement. |
I created vishal423#1. It seems to fix MongoDB, but still has issues. |
Replace MongoDB Driver with quarkus-mongodb-client
I have merged the |
We are now good on tests front except |
@vishal423 I was able to fix the MongoDB compilation error. However, tests still fail so it seems whatever authentication mechanism the tests use is not working. |
@vishal423 I'm OK with this. |
@mraible, this PR is now ready to be reviewed. During testing, I noticed a few broken scenarios and raised a couple of issues, however much more can be identified if regressed further. I will raise couple more issues that I noticed recently. |
I should have time to test in the next 24 hours. I'm currently at JavaOne, so things are a bit hectic.On Oct 17, 2022, at 13:50, Vishal Mahajan ***@***.***> wrote:
@mraible, this PR is now ready to be reviewed. During testing, I noticed a few broken scenarios and raised a couple of issues, however much more can be identified if regressed further. I will raise couple more issues that I noticed recently.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
@vishal423 I tried again with the
If I go to {
"status": 401,
"title": "Unauthorized",
"instance": "/oauth2/authorization/oidc"
} |
#248 is to track this issue. One workaround is to remove the problem library and associated code references (that I used to test OIDC flow). |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
I will merge by tomorrow. @mraible, I assume you got chance to verify all issues closed as part of it. |
@vishal423 approved |
Closes #245
Closes #213
Closes #228
Closes #222