You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "mlsorensen (via GitHub)" <gi...@apache.org> on 2023/10/04 05:07:22 UTC
[PR] Add plugin dir to developer jetty:run classpath for plugin dev [cloudstack]
mlsorensen opened a new pull request, #8033:
URL: https://github.com/apache/cloudstack/pull/8033
### Description
This PR is a simple change to allow adding plugins to CloudStack when running in developer mode. Simply drop plugin jar into `client/target/plugins` before launching `mvn -pl :cloud-client-ui jetty:run`.
Also bumping the version of jetty-maven-plugin to the same as main jetty server.
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [x] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [x] Trivial
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add plugin dir to developer jetty:run classpath for plugin dev [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #8033:
URL: https://github.com/apache/cloudstack/pull/8033#issuecomment-1782346782
@mlsorensen smoketests (github actions) are still failing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add plugin dir to developer jetty:run classpath for plugin dev [cloudstack]
Posted by "mlsorensen (via GitHub)" <gi...@apache.org>.
mlsorensen commented on code in PR #8033:
URL: https://github.com/apache/cloudstack/pull/8033#discussion_r1346190709
##########
pom.xml:
##########
@@ -154,7 +154,7 @@
<cs.jaxws.version>2.3.2-1</cs.jaxws.version>
<cs.jersey-client.version>2.26</cs.jersey-client.version>
<cs.jetty.version>9.4.51.v20230217</cs.jetty.version>
- <cs.jetty-maven-plugin.version>9.4.27.v20200227</cs.jetty-maven-plugin.version>
+ <cs.jetty-maven-plugin.version>9.4.51.v20230217</cs.jetty-maven-plugin.version>
Review Comment:
Ok I think maybe I found the issue @rohityadavcloud. Newer jetty-maven-plugin versions add the pom dependencies to classpath automatically, with older jetty we have defined `extraClasspath` and add a built cloud-client-ui.jar location. This results in a lot of duplicate classes discovered. Also notice that due to using the pom dependencies for building classpath, without specifying `-Psimulator` the jetty:run doesn't have simulator available which breaks the simulator CI build.
We can add explicit simulator dependency for the jetty-maven-plugin, or we can add `-Psimulator` to the CI (but I don't know how to do the latter).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add plugin dir to developer jetty:run classpath for plugin dev [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8033:
URL: https://github.com/apache/cloudstack/pull/8033#issuecomment-3645513860
@mlsorensen @rohityadavcloud , closing this one as it has conflicts and is old. please update and reopen if you think it is still relevant.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add plugin dir to developer jetty:run classpath for plugin dev [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on code in PR #8033:
URL: https://github.com/apache/cloudstack/pull/8033#discussion_r1345284168
##########
pom.xml:
##########
@@ -154,7 +154,7 @@
<cs.jaxws.version>2.3.2-1</cs.jaxws.version>
<cs.jersey-client.version>2.26</cs.jersey-client.version>
<cs.jetty.version>9.4.51.v20230217</cs.jetty.version>
- <cs.jetty-maven-plugin.version>9.4.27.v20200227</cs.jetty-maven-plugin.version>
+ <cs.jetty-maven-plugin.version>9.4.51.v20230217</cs.jetty-maven-plugin.version>
Review Comment:
The 9.4.27 was a known/working version, I'm not sure if with more recent working the jetty-maven-plugin works @mlsorensen have you tested it locally (see most Github actions jobs are failing)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add plugin dir to developer jetty:run classpath for plugin dev [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland closed pull request #8033: Add plugin dir to developer jetty:run classpath for plugin dev
URL: https://github.com/apache/cloudstack/pull/8033
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add plugin dir to developer jetty:run classpath for plugin dev [cloudstack]
Posted by "mlsorensen (via GitHub)" <gi...@apache.org>.
mlsorensen commented on code in PR #8033:
URL: https://github.com/apache/cloudstack/pull/8033#discussion_r1346110626
##########
pom.xml:
##########
@@ -154,7 +154,7 @@
<cs.jaxws.version>2.3.2-1</cs.jaxws.version>
<cs.jersey-client.version>2.26</cs.jersey-client.version>
<cs.jetty.version>9.4.51.v20230217</cs.jetty.version>
- <cs.jetty-maven-plugin.version>9.4.27.v20200227</cs.jetty-maven-plugin.version>
+ <cs.jetty-maven-plugin.version>9.4.51.v20230217</cs.jetty-maven-plugin.version>
Review Comment:
It worked locally, and it's the version of jetty that is bundled with the production cloudstack, but perhaps it is incompatible with the testing/PR process in some way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add plugin dir to developer jetty:run classpath for plugin dev [cloudstack]
Posted by "mlsorensen (via GitHub)" <gi...@apache.org>.
mlsorensen commented on code in PR #8033:
URL: https://github.com/apache/cloudstack/pull/8033#discussion_r1346230123
##########
pom.xml:
##########
@@ -154,7 +154,7 @@
<cs.jaxws.version>2.3.2-1</cs.jaxws.version>
<cs.jersey-client.version>2.26</cs.jersey-client.version>
<cs.jetty.version>9.4.51.v20230217</cs.jetty.version>
- <cs.jetty-maven-plugin.version>9.4.27.v20200227</cs.jetty-maven-plugin.version>
+ <cs.jetty-maven-plugin.version>9.4.51.v20230217</cs.jetty-maven-plugin.version>
Review Comment:
Eh, I'm not confident that I know exactly what's going on here, but I think the same [PR](https://github.com/eclipse/jetty.project/pull/5131) in jetty that I need for this feature is probably the same one that cloudstack's use of jetty:run is incompatible with. Or certainly the very next release after this.
May need to resolve the underlying reason why we can't upgrade to newer jetty-maven-plugin before we consider adding a plugin dir for devs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add plugin dir to developer jetty:run classpath for plugin dev [cloudstack]
Posted by "mlsorensen (via GitHub)" <gi...@apache.org>.
mlsorensen commented on code in PR #8033:
URL: https://github.com/apache/cloudstack/pull/8033#discussion_r1346190709
##########
pom.xml:
##########
@@ -154,7 +154,7 @@
<cs.jaxws.version>2.3.2-1</cs.jaxws.version>
<cs.jersey-client.version>2.26</cs.jersey-client.version>
<cs.jetty.version>9.4.51.v20230217</cs.jetty.version>
- <cs.jetty-maven-plugin.version>9.4.27.v20200227</cs.jetty-maven-plugin.version>
+ <cs.jetty-maven-plugin.version>9.4.51.v20230217</cs.jetty-maven-plugin.version>
Review Comment:
Ok I think maybe I found the issue @rohityadavcloud. Newer jetty-maven-plugin versions add the pom dependencies to class path automatically, with older jetty we have defined `extraClassPath` and add a built cloud-client-ui.jar location. This results in a lot of duplicate classes discovered. Also notice that without specifying `-Psimulator` the jetty:run doesn't have simulator available which breaks the simulator CI build.
We can add explicit simulator dependency for the jetty-maven-plugin, or we can add `-Psimulator` to the CI (but I don't know how to do the latter).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add plugin dir to developer jetty:run classpath for plugin dev [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #8033:
URL: https://github.com/apache/cloudstack/pull/8033#issuecomment-1792367668
@mlsorensen I think this is incompatible version and may require some additional changes to make them work. Does it work for you locally?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add plugin dir to developer jetty:run classpath for plugin dev [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #8033:
URL: https://github.com/apache/cloudstack/pull/8033#issuecomment-1746153547
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/8033?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#8033](https://app.codecov.io/gh/apache/cloudstack/pull/8033?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (7c3676e) into [main](https://app.codecov.io/gh/apache/cloudstack/commit/2e9b3d8e084b5f6ddf4690afe266e0a2b3f751bb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (2e9b3d8) will **decrease** coverage by `23.72%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## main #8033 +/- ##
============================================
- Coverage 28.57% 4.86% -23.72%
============================================
Files 5100 342 -4758
Lines 358565 25689 -332876
Branches 52316 4418 -47898
============================================
- Hits 102464 1250 -101214
+ Misses 241968 24305 -217663
+ Partials 14133 134 -13999
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/8033/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [simulator-marvin-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8033/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/8033/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.86% <ø> (ø)` | |
| [unit-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8033/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
[see 4758 files with indirect coverage changes](https://app.codecov.io/gh/apache/cloudstack/pull/8033/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add plugin dir to developer jetty:run classpath for plugin dev [cloudstack]
Posted by "mlsorensen (via GitHub)" <gi...@apache.org>.
mlsorensen commented on code in PR #8033:
URL: https://github.com/apache/cloudstack/pull/8033#discussion_r1346143506
##########
pom.xml:
##########
@@ -154,7 +154,7 @@
<cs.jaxws.version>2.3.2-1</cs.jaxws.version>
<cs.jersey-client.version>2.26</cs.jersey-client.version>
<cs.jetty.version>9.4.51.v20230217</cs.jetty.version>
- <cs.jetty-maven-plugin.version>9.4.27.v20200227</cs.jetty-maven-plugin.version>
+ <cs.jetty-maven-plugin.version>9.4.51.v20230217</cs.jetty-maven-plugin.version>
Review Comment:
Seems like the new versions of jetty may be more aggressive at scanning? I see more duplicate classes found, seems to be searching current directory as well as finding them in `client/target/cloud-client-ui` jar. Will try a few things...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add plugin dir to developer jetty:run classpath for plugin dev [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8033:
URL: https://github.com/apache/cloudstack/pull/8033#issuecomment-3409930843
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add plugin dir to developer jetty:run classpath for plugin dev [cloudstack]
Posted by "mlsorensen (via GitHub)" <gi...@apache.org>.
mlsorensen commented on code in PR #8033:
URL: https://github.com/apache/cloudstack/pull/8033#discussion_r1346110626
##########
pom.xml:
##########
@@ -154,7 +154,7 @@
<cs.jaxws.version>2.3.2-1</cs.jaxws.version>
<cs.jersey-client.version>2.26</cs.jersey-client.version>
<cs.jetty.version>9.4.51.v20230217</cs.jetty.version>
- <cs.jetty-maven-plugin.version>9.4.27.v20200227</cs.jetty-maven-plugin.version>
+ <cs.jetty-maven-plugin.version>9.4.51.v20230217</cs.jetty-maven-plugin.version>
Review Comment:
It worked locally, and it's the version of jetty that is bundled with the production cloudstack, but perhaps there is something else that doesn't work?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add plugin dir to developer jetty:run classpath for plugin dev [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland closed pull request #8033: Add plugin dir to developer jetty:run classpath for plugin dev
URL: https://github.com/apache/cloudstack/pull/8033
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add plugin dir to developer jetty:run classpath for plugin dev [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud closed pull request #8033: Add plugin dir to developer jetty:run classpath for plugin dev
URL: https://github.com/apache/cloudstack/pull/8033
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org