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

Draft: Setup for improving load times of Hawtio Online #801

Closed
wants to merge 2 commits into from

Conversation

mmuzikar
Copy link
Contributor

@mmuzikar mmuzikar commented Mar 4, 2024

Exposed UI components that could be used for unified look of the app (I will use HawtioLoadingPage in Hawtio Online).
Lazy loading Camel models, the fetch starts on the module load and since the both model packages are not included in the resulting bundle it significantly improves the load times.

@hawtio-ci
Copy link

hawtio-ci bot commented Mar 4, 2024

Test results

Run attempt: 1196
Detailed summary

NAME TESTS PASSED ✅ SKIPPED 💤 FAILED ❌ ERRORS 🚫 TIME 🕖
results-quarkus-node(18)-java(11)-firefox 59 0 0 59 0 1,238.771
results-quarkus-node(18)-java(17)-firefox 59 0 0 59 0 1,235.298
results-quarkus-node(20)-java(11)-firefox 59 0 0 59 0 1,237.521
results-quarkus-node(20)-java(17)-firefox 59 0 0 59 0 1,236.281
results-springboot-node(18)-java(11)-firefox 58 0 0 58 0 1,215.895
results-springboot-node(18)-java(17)-firefox 58 0 0 58 0 1,214.491
results-springboot-node(20)-java(11)-firefox 58 0 0 58 0 1,215.483
results-springboot-node(20)-java(17)-firefox 58 0 0 58 0 1,213.577

Copy link

github-actions bot commented Mar 4, 2024

Test Results

  8 files  ±0  8 suites  ±0   2h 43m 27s ⏱️ + 2h 1m 48s
 59 tests ±0  0 ✅  -  58  0 💤  - 1   59 ❌ + 59 
468 runs  ±0  0 ✅  - 460  0 💤  - 8  468 ❌ +468 

For more details on these failures, see this check.

Results for commit 452f044. ± Comparison against base commit 347d747.

♻️ This comment has been updated with latest results.

@mmuzikar mmuzikar changed the title Setup for improving load times of Hawtio Online Draft: Setup for improving load times of Hawtio Online Mar 4, 2024
@@ -333,6 +335,7 @@ export function getCamelVersions(): string[] {
* the given node. Currently, it supports Camel v3 and v4.
*/
export function getCamelModel(node: MBeanNode): CamelModel {
const { camel4, camel3 } = fetchCamelModules()
Copy link
Member

Choose a reason for hiding this comment

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

When running I get an error that camel3 is null

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, rather they should be Promise so that they will never return null.

@@ -8,6 +8,7 @@ export * from './core'
export * from './help'
export * from './plugins'
export * from './preferences'
export * from './ui'
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to export ui?

@@ -333,6 +335,7 @@ export function getCamelVersions(): string[] {
* the given node. Currently, it supports Camel v3 and v4.
*/
export function getCamelModel(node: MBeanNode): CamelModel {
const { camel4, camel3 } = fetchCamelModules()
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, rather they should be Promise so that they will never return null.

@@ -23,6 +21,9 @@ import {
} from './globals'
import { ROUTE_OPERATIONS } from './routes-service'

let _camel3: CamelModel | null = null
let _camel4: CamelModel | null = null
Copy link
Member

Choose a reason for hiding this comment

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

so if we change them to Promise them can be const not let.

camel3: _camel3!,
camel4: _camel4!,
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I think this async -> await doesn't work as you expect. Async call is blocked only inside if, which leads to null pointer exception as Paul mentioned because the if block finishes immediately and tries to return the values no matter what values _camel3 and _camel4 have.

export * from './icons'
export * from './login'
export * from './notification'
export * from './page'
Copy link
Member

Choose a reason for hiding this comment

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

Why are they needed? Note that it will change the Hawtio API we expose to external plugin authors. So generally it needs to be justified with a clear reason.

@tadayosi
Copy link
Member

tadayosi commented Mar 5, 2024

@mmuzikar Also, this pull req should be intended to fix #754. If so, please don't forget to mention that it fixes #754.

@tadayosi
Copy link
Member

tadayosi commented May 7, 2024

Should be fixed with #898. Let's close it.

@tadayosi tadayosi closed this May 7, 2024
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.

3 participants