You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/09/20 21:23:48 UTC
[GitHub] [nifi] sardell opened a new pull request, #6437: NIFI-10497: UI updates for making RegistryClient an extension point
sardell opened a new pull request, #6437:
URL: https://github.com/apache/nifi/pull/6437
This PR contains UI changes around Add/Edit Registry dialogs as a result of changes in https://github.com/apache/nifi/pull/6433.
A quick summary of the work in this PR:
* Previously, we utilized one dialog for both adding and editing a registry. Because we need to support adding/editing/deleting properties, the two dialogs have much less in common now. I've separated the dialogs into two distinct .jsp files.
* I removed the url input in the Add Registry dialog and added a combo dropdown for class type selection.
* I added tabs to the Edit Registry dialog. One of these tabs contains a table for adding/editing properties. I'm currently waiting to see if #6433 will support dynamic properties. If it doesn't, I'll need to update this work to conditionally show/hide the add property button. It currently does not work in this PR.
* This PR contains all the changes from #6433. I'm unable to start NiFi with latest work on that branch due to some java errors, which @simonbence is aware of. I'm going to open this PR as a draft until that's resolved.
# Summary
[NIFI-10497](https://issues.apache.org/jira/browse/NIFI-10497)
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [ ] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- [ ] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
- [ ] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
### Pull Request Formatting
- [ ] Pull Request based on current revision of the `main` branch
- [ ] Pull Request refers to a feature branch with one commit containing changes
# Verification
Please indicate the verification steps performed prior to pull request creation.
### Build
- [ ] Build completed using `mvn clean install -P contrib-check`
- [ ] JDK 8
- [ ] JDK 11
- [ ] JDK 17
### Licensing
- [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [ ] Documentation formatting appears as expected in rendered files
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] sardell commented on a diff in pull request #6437: NIFI-10497: UI updates for making RegistryClient an extension point
Posted by GitBox <gi...@apache.org>.
sardell commented on code in PR #6437:
URL: https://github.com/apache/nifi/pull/6437#discussion_r977719615
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/WEB-INF/partials/canvas/registry-configuration-dialog.jsp:
##########
@@ -15,26 +15,45 @@
limitations under the License.
--%>
<%@ page contentType="text/html" pageEncoding="UTF-8" session="false" %>
-<div id="registry-configuration-dialog" layout="column" class="hidden medium-dialog">
+<div id="registry-configuration-dialog" layout="column" class="hidden large-dialog">
+ <div>
+ <div class="clear"></div>
+ </div>
<div class="dialog-content">
- <div class="setting">
- <div class="setting-name">Name</div>
- <div class="setting-field">
- <span id="registry-id" class="hidden"></span>
- <input type="text" id="registry-name" class="setting-input"/>
- </div>
- </div>
- <div class="setting">
- <div class="setting-name">URL</div>
- <div class="setting-field">
- <input type="text" id="registry-location" class="setting-input" placeholder="https://remotehost:8443"/>
+ <div id="registry-configuration-tabs" class="registration-config-tabs tab-container"></div>
+ <div id="registries-tabs-content">
+ <div id="registry-configuration-settings-tab-content">
+ <div id="registry-fields">
+ <div class="setting">
+ <div class="setting-name">Id</div>
+ <div class="setting-field">
+ <span id="registry-id-config"></span>
+ </div>
+ </div>
+ <div class="setting">
+ <div class="setting-name">Name</div>
+ <div class="setting-field">
+ <span id="registry-name-config"></span>
Review Comment:
That was my misunderstanding. @simonbence just pointed this out to me as well.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mcgilman merged pull request #6437: NIFI-10538: UI updates for making RegistryClient an extension point
Posted by GitBox <gi...@apache.org>.
mcgilman merged PR #6437:
URL: https://github.com/apache/nifi/pull/6437
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mcgilman commented on pull request #6437: NIFI-10497: UI updates for making RegistryClient an extension point
Posted by GitBox <gi...@apache.org>.
mcgilman commented on PR #6437:
URL: https://github.com/apache/nifi/pull/6437#issuecomment-1252931281
Will review...
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mcgilman commented on a diff in pull request #6437: NIFI-10538: UI updates for making RegistryClient an extension point
Posted by GitBox <gi...@apache.org>.
mcgilman commented on code in PR #6437:
URL: https://github.com/apache/nifi/pull/6437#discussion_r979979750
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-controller-service.js:
##########
@@ -257,10 +257,12 @@
// reload
nfParameterProvider.reload(reference.id);
+ } else if (reference.referenceType === 'FlowRegistryClient') {
Review Comment:
I think something may have been mixed up during a rebase here.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-controller-service.js:
##########
@@ -402,6 +404,36 @@
});
};
+ var updateValidationErrors = function (validationErrorIcon, referencingComponent) {
Review Comment:
This function appears to be a duplicate. Likely another mixed up with the rebase. Also, there's another condition that may have been missed. Search for 'ParameterProvider' to find other instances.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-flow-version.js:
##########
@@ -206,7 +206,8 @@
registries.push({
text: registry.name,
value: registry.id,
- description: nfCommon.escapeHtml(registry.description)
+ description: nfCommon.escapeHtml(registry.description),
+ disabled: registry.validationStatus !== 'VALID' // TODO: verify this logic is correct
Review Comment:
This looks correct but we should check with @simonbence or @markap14 who are a little more familiar with the backend effort [1].
[1] #6433
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mcgilman commented on a diff in pull request #6437: NIFI-10497: UI updates for making RegistryClient an extension point
Posted by GitBox <gi...@apache.org>.
mcgilman commented on code in PR #6437:
URL: https://github.com/apache/nifi/pull/6437#discussion_r977684323
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/WEB-INF/partials/canvas/registry-configuration-dialog.jsp:
##########
@@ -15,26 +15,45 @@
limitations under the License.
--%>
<%@ page contentType="text/html" pageEncoding="UTF-8" session="false" %>
-<div id="registry-configuration-dialog" layout="column" class="hidden medium-dialog">
+<div id="registry-configuration-dialog" layout="column" class="hidden large-dialog">
+ <div>
+ <div class="clear"></div>
+ </div>
<div class="dialog-content">
- <div class="setting">
- <div class="setting-name">Name</div>
- <div class="setting-field">
- <span id="registry-id" class="hidden"></span>
- <input type="text" id="registry-name" class="setting-input"/>
- </div>
- </div>
- <div class="setting">
- <div class="setting-name">URL</div>
- <div class="setting-field">
- <input type="text" id="registry-location" class="setting-input" placeholder="https://remotehost:8443"/>
+ <div id="registry-configuration-tabs" class="registration-config-tabs tab-container"></div>
+ <div id="registries-tabs-content">
+ <div id="registry-configuration-settings-tab-content">
+ <div id="registry-fields">
+ <div class="setting">
+ <div class="setting-name">Id</div>
+ <div class="setting-field">
+ <span id="registry-id-config"></span>
+ </div>
+ </div>
+ <div class="setting">
+ <div class="setting-name">Name</div>
+ <div class="setting-field">
+ <span id="registry-name-config"></span>
Review Comment:
I think that `name` should be configurable here.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/WEB-INF/partials/canvas/registry-configuration-dialog.jsp:
##########
@@ -15,26 +15,45 @@
limitations under the License.
--%>
<%@ page contentType="text/html" pageEncoding="UTF-8" session="false" %>
-<div id="registry-configuration-dialog" layout="column" class="hidden medium-dialog">
+<div id="registry-configuration-dialog" layout="column" class="hidden large-dialog">
+ <div>
+ <div class="clear"></div>
+ </div>
<div class="dialog-content">
- <div class="setting">
- <div class="setting-name">Name</div>
- <div class="setting-field">
- <span id="registry-id" class="hidden"></span>
- <input type="text" id="registry-name" class="setting-input"/>
- </div>
- </div>
- <div class="setting">
- <div class="setting-name">URL</div>
- <div class="setting-field">
- <input type="text" id="registry-location" class="setting-input" placeholder="https://remotehost:8443"/>
+ <div id="registry-configuration-tabs" class="registration-config-tabs tab-container"></div>
+ <div id="registries-tabs-content">
+ <div id="registry-configuration-settings-tab-content">
+ <div id="registry-fields">
+ <div class="setting">
+ <div class="setting-name">Id</div>
+ <div class="setting-field">
+ <span id="registry-id-config"></span>
+ </div>
+ </div>
+ <div class="setting">
+ <div class="setting-name">Name</div>
+ <div class="setting-field">
+ <span id="registry-name-config"></span>
+ </div>
+ </div>
+ <div class="setting">
+ <div class="setting-name">Type</div>
+ <div class="setting-field">
+ <span id="registry-type-config"></span>
+ </div>
+ </div>
+ <div class="setting">
+ <div class="setting-name">Description</div>
+ <div class="setting-field">
+ <span id="registry-description-config"></span>
Review Comment:
I think `description` should be configurable here.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-settings.js:
##########
@@ -1674,6 +1766,89 @@
});
};
+ /**
+ * Loads available registry types.
+ */
+ var loadRegistryTypes = function () {
+ return $.ajax({
+ type: 'GET',
+ url: config.urls.registryTypes,
+ dataType: 'json'
+ }).done(function (response) {
+ var regTypeOptions = [];
+ response.flowRegistryClientTypes.forEach((type) => {
+ regTypeOptions.push({
+ text: nfCommon.substringAfterLast(type.type, '.'),
+ value: type.type,
+ description: type.description || ''
+ })
+ });
+
+ $('#new-registry-type-combo').combo({
+ options: regTypeOptions
+ });
+ }
+ )};
+
+ /**
+ * Determines whether the user has made any changes to the registry configuration
+ * that needs to be saved.
+ */
+ var isSaveRequired = function () {
+ return $('#registry-properties').propertytable('isSaveRequired');
Review Comment:
Once `name` and `description` are configurable, we'll need to consider them here as well to know if a save is required. Also, `updateRegistry` will need to include them in the request payload. It appears the name is already included there.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-processor-configuration.js:
##########
@@ -1096,6 +1096,8 @@
// set the button model
$('#processor-configuration').modal('setButtonModel', buttons);
+ console.log('processor.config.properties', processor.config.properties);
+ console.log('processor.config.descriptors', processor.config.descriptors);
Review Comment:
Reminder to remove these debugging lines before marking the PR as ready.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-settings.js:
##########
@@ -1464,11 +1540,18 @@
* @param registryEntity
*/
var editRegistry = function (registryEntity) {
+ // get the controller service history
+ var loadConfig = $.ajax({
+ type: 'GET',
+ url: '../nifi-api/controller/registry-clients/' + encodeURIComponent(registryEntity.id),
+ dataType: 'json'
+ }).done();
Review Comment:
I would suggest putting the rest of the method into the done callback here. This is the typical convention in NiFi. It also ensures we have the latest version of the registry client. As-is the properties are "fresh" but the "name" could be stale.
More interestingly, the revision could be stale too. In `updateRegistry` the entire entity is pulled out of the grid. We'd probably want to `updateItem` for the freshly fetched `registryEntity` or change the signature of `updateItem` to accept a `registryEntity` which can be supplied here.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mcgilman commented on a diff in pull request #6437: NIFI-10538: UI updates for making RegistryClient an extension point
Posted by GitBox <gi...@apache.org>.
mcgilman commented on code in PR #6437:
URL: https://github.com/apache/nifi/pull/6437#discussion_r980008876
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-controller-service.js:
##########
@@ -670,6 +702,44 @@
var providerItem = $('<li></li>').append(providerState).append(providerBulletins).append(parameterProviderLink).append(providerType);
providers.append(providerItem);
+ } else if (referencingComponent.referenceType === 'FlowRegistryClient') {
+ var registryLink = $('<span class="referencing-component-name link"></span>').text(referencingComponent.name).on('click', function () {
+ var registryGrid = $('#registries-table').data('gridInstance');
+ var registryData = registryGrid.getData();
+
+ // select the selected row
+ var row = registryData.getRowById(referencingComponent.id);
+ registryGrid.setSelectedRows([row]);
+ registryGrid.scrollRowIntoView(row);
+
+ // select the reporting task tab
+ $('#settings-tabs').find('li:nth-child(4)').click();
+
+ // close the dialog and shell
+ referenceContainer.closest('.dialog').modal('hide');
+ });
+
+ // registry state - used to show the validation errors
+ var registryState = $('<div class="referencing-component-state invalid"></div>').addClass(referencingComponent.id + 'state');
+
+ if (nfCommon.isEmpty(referencingComponent.validationErrors)) {
+ registryState.hide();
+ } else {
+ updateValidationErrors(registryState, referencingComponent);
+ }
+
+ // type
+ var registryType = $('<span class="referencing-component-type"></span>').text(nfCommon.substringAfterLast(referencingComponent.type, '.'));
+
+ // active thread count
+ var registryActiveThreadCount = $('<span class="referencing-component-active-thread-count"></span>').addClass(referencingComponent.id + '-active-threads');
+ if (nfCommon.isDefinedAndNotNull(referencingComponent.activeThreadCount) && referencingComponent.activeThreadCount > 0) {
+ registryActiveThreadCount.text('(' + referencingComponent.activeThreadCount + ')');
+ }
+
+ // registry item
+ var registryItem = $('<li></li>').append(registryState).append(registryLink).append(registryType).append(registryActiveThreadCount);
+ tasks.append(registryItem);
Review Comment:
These registry clients are being added to the Reporting Tasks section. Please create a dedicated section to Flow Registry Clients.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-controller-service.js:
##########
@@ -670,6 +702,44 @@
var providerItem = $('<li></li>').append(providerState).append(providerBulletins).append(parameterProviderLink).append(providerType);
providers.append(providerItem);
+ } else if (referencingComponent.referenceType === 'FlowRegistryClient') {
+ var registryLink = $('<span class="referencing-component-name link"></span>').text(referencingComponent.name).on('click', function () {
+ var registryGrid = $('#registries-table').data('gridInstance');
+ var registryData = registryGrid.getData();
+
+ // select the selected row
+ var row = registryData.getRowById(referencingComponent.id);
+ registryGrid.setSelectedRows([row]);
+ registryGrid.scrollRowIntoView(row);
+
+ // select the reporting task tab
+ $('#settings-tabs').find('li:nth-child(4)').click();
+
+ // close the dialog and shell
+ referenceContainer.closest('.dialog').modal('hide');
+ });
+
+ // registry state - used to show the validation errors
+ var registryState = $('<div class="referencing-component-state invalid"></div>').addClass(referencingComponent.id + 'state');
+
+ if (nfCommon.isEmpty(referencingComponent.validationErrors)) {
+ registryState.hide();
+ } else {
+ updateValidationErrors(registryState, referencingComponent);
+ }
+
+ // type
+ var registryType = $('<span class="referencing-component-type"></span>').text(nfCommon.substringAfterLast(referencingComponent.type, '.'));
+
+ // active thread count
+ var registryActiveThreadCount = $('<span class="referencing-component-active-thread-count"></span>').addClass(referencingComponent.id + '-active-threads');
+ if (nfCommon.isDefinedAndNotNull(referencingComponent.activeThreadCount) && referencingComponent.activeThreadCount > 0) {
+ registryActiveThreadCount.text('(' + referencingComponent.activeThreadCount + ')');
+ }
+
+ // registry item
+ var registryItem = $('<li></li>').append(registryState).append(registryLink).append(registryType).append(registryActiveThreadCount);
+ tasks.append(registryItem);
Review Comment:
These registry clients are being added to the Reporting Tasks section. Please create a dedicated section for Flow Registry Clients.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org