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/22 14:06:08 UTC
[GitHub] [nifi] mcgilman commented on a diff in pull request #6437: NIFI-10497: UI updates for making RegistryClient an extension point
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