-
Notifications
You must be signed in to change notification settings - Fork 185
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
kie-issues#1443: Change JBPM DevUI to use current host origin to generate GraphQL URL to be consumed by the frontend #2536
Conversation
packages/runtime-tools-process-dev-ui-webapp/src/components/DevUI/RuntimeTools/RuntimeTools.tsx
Outdated
Show resolved
Hide resolved
packages/runtime-tools-process-dev-ui-webapp/src/components/contexts/DevUIAppContext.tsx
Outdated
Show resolved
Hide resolved
packages/runtime-tools-process-dev-ui-webapp/src/envelope/RuntimeToolsDevUIEnvelopeApiImpl.ts
Outdated
Show resolved
Hide resolved
String devUIUrl = getProperty(configurationBuildItem, systemPropertyBuildItems, "kogito.dev-ui.url"); | ||
String dataIndexUrl = getProperty(configurationBuildItem, systemPropertyBuildItems, "kogito.data-index.url"); | ||
String dataIndexUrl = getProperty(configurationBuildItem, systemPropertyBuildItems, "kogito.dataindex.http.url"); |
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.
Not sure I understand this very well. We're changing our public API? What other places do we need to update? Maybe the examples?
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.
Both kogito.dataindex.http.url
and kogito-data-index.url
settings exist, idk why.
I had more success using kogito.dataindex.http.url
, and from checking the source code in kogito-apps this should be the one we use.
@pefernan might know better.
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.
@thiagoelg @tiagobento sorry for being late here, I missed this while on PTO.
This System property (kogito.data-index.url
) is the one that was set by default by the Runtime Dev Services if it starts the Embedded Data-Index and this same property is set by default by the Data Index Addon if present. So that's why I made the DevProcesor use it.
The kogito.dataindex.http.url
as far as I know is a config property that is being used by certain components (So far I only found the SVG addon) to build WebClients to interact with Data Index or in IT Tests. IDK if it has any other internal usage.
In this particular case I'd keep using the original property (kogito.data-index.url
) but I think it would be worth checking if we could consolidate the usage of the data-index url in a single system property instead of having two of them.
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.
lgtm
…generate GraphQL URL to be consumed by the frontend (apache#2536)
JBPM DevUI will now use the current host origin for all URLs unless configured otherwise.
Fixes apache/incubator-kie-issues#1443
This PR also fixes apache/incubator-kie-issues#1275 for the JBPM DevUI (Sonataflow DevUI was not changed)
Screen.Recording.2024-08-16.at.17.44.05.mov