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 2021/04/19 18:50:53 UTC
[GitHub] [nifi-registry] mtien-apache opened a new pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
mtien-apache opened a new pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319
Thank you for submitting a contribution to Apache NiFi Registry.
Please provide a short description of the PR here:
#### Description of PR
- [WIP] This PR adds the ability for a user to take a previously saved versionedFlowSnapshot JSON file and import it as a new flow definition to an existing bucket or import it as the next new version of an existing flow through the UI.
- This PR also adds the ability to export a versionedFlowSnapshot as a JSON file, which removes snapshot metadata associated with the given Registry.
- New dialogs have been created to handle import and export.
- The REST endpoints added in `BucketFlowResource` are -
- `importFlow` - `/buckets/{bucketId}/flows/import`
- `importVersionedFlow` - `/buckets/{bucketId}/flows/{flowId}/versions/import`
- `exportVersionedFlow`- `/buckets/{bucketId}/flows/{flowId}/versions/{versionNumber: \d+}/export`
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [x] Does your PR title start with **NIFIREG-XXXX** where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [x] Has your PR been rebased against the latest commit within the target branch (typically `main`)?
- [ ] Is your initial contribution a single, squashed commit? _Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not `squash` or use `--force` when pushing to allow for clean monitoring of changes._
### For code changes:
- [ ] Have you ensured that the full suite of tests is executed via `mvn -Pcontrib-check clean install` at the root `nifi-registry` folder?
- [ ] Have you written or updated unit tests to verify your changes?
- [x] Have you verified that the full build is successful on JDK 8?
- [ ] Have you verified that the full build is successful on JDK 11?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the `LICENSE` file, including the main `LICENSE` file under `nifi-registry-assembly`?
- [ ] If applicable, have you updated the `NOTICE` file, including the main `NOTICE` file found under `nifi-registry-assembly`?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI for build issues and submit an update to your PR as soon as possible.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-823772139
I have pushed a commit that resolves the importing and exporting issues found by @bbende, as well as addressing other feedback found above this comment.
Thanks for the first reviews, @bbende and @exceptionfactory !
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r617188670
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +303,129 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Upload flow version",
+ notes = "Uploads the next version of a flow. The version number of the object being created must be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) })
+ public Response importVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier")
+ final String bucketId,
+ @PathParam("flowId")
+ @ApiParam(value = "The flow identifier")
+ final String flowId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("comments") final String comments) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ // deserialize InputStream to a VersionedFlowSnapshot
+ VersionedFlowSnapshot versionedFlowSnapshot;
+
+ versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ // clear or set the necessary snapShotMetadata
+ if (versionedFlowSnapshot.getSnapshotMetadata() != null) {
+ versionedFlowSnapshot.getSnapshotMetadata().setBucketIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setFlowIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setLink(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ // if there are new comments, then set it
+ // otherwise, keep the original comments
+ if (!StringUtils.isBlank(comments)) {
+ versionedFlowSnapshot.getSnapshotMetadata().setComments(comments);
+ }
+ }
+
+ return createFlowVersion(bucketId, flowId, versionedFlowSnapshot);
+ }
+
+ @POST
+ @Path("import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Create flow",
+ notes = "Creates a flow in the given bucket. The flow id is created by the server and populated in the returned entity.",
+ response = VersionedFlow.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}")})
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409)})
+ public Response importFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier") final String bucketId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("name") final String name,
+ @FormDataParam("description") final String description) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(name)) {
+ throw new IllegalArgumentException("The flow name is required.");
+ }
+
+ // create VersionedFlow
+ final VersionedFlow versionedFlow = new VersionedFlow();
+
+ versionedFlow.setName(name);
+ versionedFlow.setRevision(new RevisionInfo(null, 0L));
+ if (!StringUtils.isBlank(description)) {
+ versionedFlow.setDescription(description);
+ }
+
+ final VersionedFlow createdFlow = createAndPublishVersionedFlow(bucketId, versionedFlow);
+
+ // deserialize InputStream and create new VersionedFlowSnapshot
+ final VersionedFlowSnapshot versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ setSnaphotMetadataIfMissing(bucketId, createdFlow.getIdentifier(), versionedFlowSnapshot);
+
+ // set remaining snapshot metadata
+ final String userIdentity = NiFiUserUtils.getNiFiUserIdentity();
+ versionedFlowSnapshot.getSnapshotMetadata().setAuthor(userIdentity);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
Review comment:
Changed to use a static variable.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] andrewmlim commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
andrewmlim commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-831650739
>
> Although I agree the error message could be better, it at least points to which line the error is found. I believe this is sufficient for the time being. Thoughts?
I agree that it is sufficient for now. All of the error messages can be handled outside of this PR. Thanks!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r617187404
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -385,6 +520,64 @@ public Response getLatestFlowVersionMetadata(
return Response.status(Response.Status.OK).entity(latest).build();
}
+ @GET
+ @Path("{flowId}/versions/{versionNumber: \\d+}/export")
+ @Consumes(MediaType.WILDCARD)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Exports specified bucket flow version content",
+ notes = "Exports the specified version of a flow, including the metadata and content of the flow.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "read"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}")})
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409)})
+ public Response exportVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier") final String bucketId,
+ @PathParam("flowId")
+ @ApiParam("The flow identifier") final String flowId,
+ @PathParam("versionNumber")
+ @ApiParam("The version number") final Integer versionNumber) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ if (versionNumber == null) {
+ throw new IllegalArgumentException("The version number is required.");
+ }
+
+ final VersionedFlowSnapshot versionedFlowSnapshot = serviceFacade.getFlowSnapshot(bucketId, flowId, versionNumber);
Review comment:
I refactored the some of this logic into the service facade.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-825935715
@andrewmlim Thanks for reviewing!
- The UI changes make sense to me, especially if the names will match what's in the documentation and NiFi.
@moranr Do you have any additional thoughts before I proceed?
- For the "DELETE" flow button color in the 'Delete Flow' dialog is what's currently in Registry:
> * I noticed that the color of the "DELETE" button in the "Delete Flow" dialog is a different color (more red) than the other dialog buttons (IMPORT and DOWNLOAD). This appears to be intentional. The "DELETE" button for deleting a bucket is the same red, so understandable if the color difference is to make it stand out since it is a unrecoverable action.
- I'm looking into the invalid flow definition file error now.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] andrewmlim commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
andrewmlim commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-830366080
> Related to my last comment, here are the messages in the error dialogs I have found so far:
>
> * Non flow definition JSON file: "Cannot create versioned flow snapshot - flowContents must not be null"
> * Incomplete flow definition JSON file: "Deserialization of uploaded JSON failed"
>
> It would be great if we could make these error messages more user friendly. But that could definitely be done as a separate Jira/PR. I wouldn't block this PR to address this.
For the incomplete flow definition JSON file, the error I see now when importing is: "Unexpected end-of-input: expected close marker for Object (start marker at [Source: (org.glassfish.jersey.message.internal.ReaderInterceptorExecutor$UnCloseableInputStream); line: 1, column: 1]) at [Source: (org.glassfish.jersey.message.internal.ReaderInterceptorExecutor$UnCloseableInputStream); line: 2, column: 1]"
which is more specific to the issue (that there is no closing } in the JSON), but less readable.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-849866685
Closed in favor of https://github.com/apache/nifi/pull/5107
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] andrewmlim commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
andrewmlim commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-825773747
Additional thoughts:
- In the "Import New Flow" and "Import New Version" dialogs, change "Data Flow Name" to "Flow Name" for same reasons stated earlier.
- In the "Import New Version" dialog, consider changing "Change Log Comment" to "Version Comments" which is what is used in NiFi when saving a flow version.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r617188536
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +303,129 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Upload flow version",
+ notes = "Uploads the next version of a flow. The version number of the object being created must be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) })
+ public Response importVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier")
+ final String bucketId,
+ @PathParam("flowId")
+ @ApiParam(value = "The flow identifier")
+ final String flowId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("comments") final String comments) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ // deserialize InputStream to a VersionedFlowSnapshot
+ VersionedFlowSnapshot versionedFlowSnapshot;
+
+ versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ // clear or set the necessary snapShotMetadata
+ if (versionedFlowSnapshot.getSnapshotMetadata() != null) {
+ versionedFlowSnapshot.getSnapshotMetadata().setBucketIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setFlowIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setLink(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
Review comment:
Good idea - I've changed to use a static variable and removed the duplicate line of code.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r617187780
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -385,6 +520,64 @@ public Response getLatestFlowVersionMetadata(
return Response.status(Response.Status.OK).entity(latest).build();
}
+ @GET
+ @Path("{flowId}/versions/{versionNumber: \\d+}/export")
Review comment:
I don't think the space is necessary, but decided to follow the format of the other endpoints.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] andrewmlim commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
andrewmlim commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-837406461
> One minor comment... when downloading a version, the window that pops up has the latest version labeled as "Latest Version (v.#)" but then the other items in the list are just "Version #", seems a little inconsistent that one has "v." and the others don't. Should the latest one just say "Latest Version (#)" or maybe "Version # (latest)" ?
>
> Also, while writing this, I started thinking about how we have generally been referring to this as import/export, and the buttons for import use the word "Import", but the for export we are calling it "Download Version". I'm not too hung up on this, but started wondering if that should be called "Export Version" to better correspond with "Import"?
I think the most consistent label would be "Latest (Version #)".
"Export Version" is a good suggestion. Like Bryan, I don't think that change is critical.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r617189439
##########
File path: nifi-registry-core/nifi-registry-web-ui/src/main/webapp/services/nf-registry.api.js
##########
@@ -75,6 +75,113 @@ NfRegistryApi.prototype = {
);
},
+ /**
+ * Retrieves the specified versioned flow snapshot for an existing droplet the registry has stored.
+ *
+ * @param {string} dropletUri The uri of the droplet to request.
+ * @param {number} versionNumber The version of the flow to request.
+ * @returns {*}
+ */
+ exportDropletVersionedSnapshot: function (dropletUri, versionNumber) {
+ var self = this;
+ var url = '../nifi-registry-api/' + dropletUri;
+ url += '/versions/' + versionNumber + '/export';
Review comment:
I don't think it's necessary to split up. I have changed to one line.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] andrewmlim commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
andrewmlim commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-825837500
Related to my last comment, here are the messages in the error dialogs I have found so far:
- Non flow definition JSON file: "Cannot create versioned flow snapshot - flowContents must not be null"
- Incomplete flow definition JSON file: "Deserialization of uploaded JSON failed"
It would be great if we could make these error messages more user friendly. But that could definitely be done as a separate Jira/PR. I wouldn't block this PR to address this.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-831591230
> > Related to my last comment, here are the messages in the error dialogs I have found so far:
> >
> > * Non flow definition JSON file: "Cannot create versioned flow snapshot - flowContents must not be null"
> > * Incomplete flow definition JSON file: "Deserialization of uploaded JSON failed"
> >
> > It would be great if we could make these error messages more user friendly. But that could definitely be done as a separate Jira/PR. I wouldn't block this PR to address this.
@andrewmlim I refactored the resource method to let the Spring framework to automatically handle the deserialization and removed the "Deserialization of uploaded JSON failed" message.
Agree to improve the error message in a separate PR for the "Cannot create versioned flow snapshot - flowContents must not be null" message.
> For the incomplete flow definition JSON file, the error I see now when importing is: "Unexpected end-of-input: expected close marker for Object (start marker at [Source: (org.glassfish.jersey.message.internal.ReaderInterceptorExecutor$UnCloseableInputStream); line: 1, column: 1]) at [Source: (org.glassfish.jersey.message.internal.ReaderInterceptorExecutor$UnCloseableInputStream); line: 2, column: 1]"
>
> which is more specific to the issue (that there is no closing } in the JSON), but less readable.
After investigating this exception message, it seems the exception is being caught by an `ExceptionMapper` and translating it to a BadRequest. It may be from `jackson-databind` in the Spring framework, which is not something we have much control over. It's possible to write a custom ExceptionMapper for a BadRequest, but the custom message could end up applying across the app for any method throwing that type of Exception, which can be addressed in a separate PR.
Although I agree the error message could be better, it at least points to which line the error is found. I believe this is sufficient for the time being. Thoughts?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache closed pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache closed pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r617187478
##########
File path: nifi-registry-core/nifi-registry-web-api/pom.xml
##########
@@ -423,6 +423,16 @@
<artifactId>jjwt</artifactId>
<version>0.7.0</version>
</dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-databind</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.protobuf</groupId>
+ <artifactId>protobuf-java</artifactId>
+ <version>3.11.4</version>
+ <scope>compile</scope>
+ </dependency>
Review comment:
Removed - see above comment.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] exceptionfactory commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r626113532
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +294,42 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.APPLICATION_JSON)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Upload flow version",
Review comment:
Should this be changed to Import flow version?
```suggestion
value = "Import flow version",
```
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +294,42 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.APPLICATION_JSON)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Upload flow version",
+ notes = "Uploads the next version of a flow. The version number of the object being created must be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
Review comment:
The notes make it sound like the version number must be specified, as opposed to being generated.
```suggestion
notes = "Import the next version of a flow. The version number of the object being created will be the " +
"next available version integer. Flow versions are immutable after they are created.",
```
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/service/StandardServiceFacade.java
##########
@@ -360,6 +362,48 @@ public VersionedFlowSnapshot getLatestFlowSnapshot(final String flowIdentifier)
return lastSnapshot;
}
+ @Override
+ public VersionedFlowSnapshot importVersionedFlowSnapshot(VersionedFlowSnapshot versionedFlowSnapshot, String bucketIdentifier,
+ String flowIdentifier, String comments) {
+ // set new snapshotMetadata
+ final VersionedFlowSnapshotMetadata metadata = new VersionedFlowSnapshotMetadata();
+ metadata.setBucketIdentifier(bucketIdentifier);
+ metadata.setFlowIdentifier(flowIdentifier);
+ metadata.setVersion(LATEST_VERSION);
+
+ // if there are new comments, then set it
+ // otherwise, keep the original comments
+ if (!StringUtils.isBlank(comments)) {
+ metadata.setComments(comments);
+ } else if (versionedFlowSnapshot.getSnapshotMetadata() != null && versionedFlowSnapshot.getSnapshotMetadata().getComments() != null) {
Review comment:
Should this line check for blank comments?
```suggestion
} else if (versionedFlowSnapshot.getSnapshotMetadata() != null && StringUtils.isNotBlank(versionedFlowSnapshot.getSnapshotMetadata().getComments())) {
```
##########
File path: nifi-registry-core/nifi-registry-web-ui/src/main/webapp/components/explorer/grid-list/dialogs/import-new-flow/nf-registry-import-new-flow.js
##########
@@ -0,0 +1,192 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { Component } from '@angular/core';
+
+import NfRegistryApi from 'services/nf-registry.api';
+import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material';
+import { FdsSnackBarService } from '@nifi-fds/core';
+
+/**
+ * NfRegistryImportNewFlow constructor.
+ *
+ * @param nfRegistryApi The api service.
+ * @param fdsSnackBarService The FDS snack bar service module.
+ * @param matDialogRef The angular material dialog ref.
+ * @param data The data passed into this component.
+ * @constructor
+ */
+function NfRegistryImportNewFlow(nfRegistryApi, fdsSnackBarService, matDialogRef, data) {
+ // Services
+ this.snackBarService = fdsSnackBarService;
+ this.nfRegistryApi = nfRegistryApi;
+ this.dialogRef = matDialogRef;
+ // local state
+ this.keepDialogOpen = false;
+ this.protocol = location.protocol;
+ this.buckets = data.buckets;
+ this.activeBucket = data.activeBucket.identifier;
+ this.writableBuckets = [];
+ this.fileToUpload = null;
+ this.fileName = null;
+ this.name = '';
+ this.description = '';
+ this.selectedBucket = {};
+ this.hoverValidity = '';
+ this.extensions = 'application/json';
+ this.multiple = false;
+}
+
+NfRegistryImportNewFlow.prototype = {
+ constructor: NfRegistryImportNewFlow,
+
+ ngOnInit: function () {
+ this.writableBuckets = this.filterWritableBuckets(this.buckets);
+
+ // if there's only 1 writable bucket, always set as the initial value in the bucket dropdown
+ // if opening the dialog from the explorer/grid-list, there is no active bucket
+ if (typeof this.activeBucket === 'undefined') {
Review comment:
Can this be simplified to check against `undefined` without `typeof`?
```suggestion
if (this.activeBucket === undefined) {
```
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +294,42 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.APPLICATION_JSON)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Upload flow version",
+ notes = "Uploads the next version of a flow. The version number of the object being created must be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) })
+ public Response importVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier")
+ final String bucketId,
+ @PathParam("flowId")
+ @ApiParam(value = "The flow identifier")
+ final String flowId,
+ @ApiParam("file") final VersionedFlowSnapshot versionedFlowSnapshot,
+ @HeaderParam("comments") final String comments) {
Review comment:
Recommend capitalizing this header to follow HTTP conventions:
```suggestion
@HeaderParam("Comments") final String comments) {
```
##########
File path: nifi-registry-core/nifi-registry-web-api/src/test/java/org/apache/nifi/registry/web/api/FlowsIT.java
##########
@@ -541,4 +544,171 @@ public void testFlowNameUniquePerBucket() throws Exception {
}
+ @Test
+ public void testImportVersionedFlowSnapshot() {
+ final RevisionInfo initialRevision = new RevisionInfo("FlowsIT", 0L);
+
+ // Create a versionedFlowSnapshot to export
+ // Given: an empty Bucket "3" (see FlowsIT.sql) with a newly created flow
+
+ final String bucketId = "3";
+ final VersionedFlow flow = new VersionedFlow();
+ flow.setBucketIdentifier(bucketId);
+ flow.setName("Test Flow for creating snapshots");
+ flow.setDescription("This is a randomly named flow created by an integration test for the purpose of holding snapshots.");
+ flow.setRevision(initialRevision);
+
+ final VersionedFlow createdFlow = client
+ .target(createURL("buckets/{bucketId}/flows"))
+ .resolveTemplate("bucketId", bucketId)
+ .request()
+ .post(Entity.entity(flow, MediaType.APPLICATION_JSON), VersionedFlow.class);
+ final String flowId = createdFlow.getIdentifier();
+
+ // Create snapshotMetadata
+ final VersionedFlowSnapshotMetadata flowSnapshotMetadata = new VersionedFlowSnapshotMetadata();
+ flowSnapshotMetadata.setBucketIdentifier(bucketId);
+ flowSnapshotMetadata.setFlowIdentifier(flowId);
+ flowSnapshotMetadata.setComments("This is a snapshot created by an integration test.");
+
+ // Create a VersionedFlowSnapshot
+ final VersionedFlowSnapshot flowSnapshot = new VersionedFlowSnapshot();
+ flowSnapshot.setSnapshotMetadata(flowSnapshotMetadata);
+ flowSnapshot.setFlowContents(new VersionedProcessGroup()); // an empty root process group
+ flowSnapshot.getFlowContents().setName("Test Flow name");
+ flowSnapshot.getSnapshotMetadata().setVersion(-1);
Review comment:
Recommend setting a private static final variable named LATEST_VERSION and reusing here and in the other test method.
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/service/StandardServiceFacade.java
##########
@@ -360,6 +362,48 @@ public VersionedFlowSnapshot getLatestFlowSnapshot(final String flowIdentifier)
return lastSnapshot;
}
+ @Override
+ public VersionedFlowSnapshot importVersionedFlowSnapshot(VersionedFlowSnapshot versionedFlowSnapshot, String bucketIdentifier,
+ String flowIdentifier, String comments) {
+ // set new snapshotMetadata
+ final VersionedFlowSnapshotMetadata metadata = new VersionedFlowSnapshotMetadata();
+ metadata.setBucketIdentifier(bucketIdentifier);
+ metadata.setFlowIdentifier(flowIdentifier);
+ metadata.setVersion(LATEST_VERSION);
+
+ // if there are new comments, then set it
+ // otherwise, keep the original comments
+ if (!StringUtils.isBlank(comments)) {
Review comment:
This could be changed to `StringUtils.isNotBlank()`
```suggestion
if (StringUtils.isNotBlank(comments)) {
```
##########
File path: nifi-registry-core/nifi-registry-web-ui/src/main/webapp/services/nf-registry.api.js
##########
@@ -75,6 +75,137 @@ NfRegistryApi.prototype = {
);
},
+ /**
+ * Retrieves the specified versioned flow snapshot for an existing droplet the registry has stored.
+ *
+ * @param {string} dropletUri The uri of the droplet to request.
+ * @param {number} versionNumber The version of the flow to request.
+ * @returns {*}
+ */
+ exportDropletVersionedSnapshot: function (dropletUri, versionNumber) {
+ var self = this;
+ var url = '../nifi-registry-api/' + dropletUri + '/versions/' + versionNumber + '/export';
+ var options = {
+ headers: headers,
+ observe: 'response'
+ };
+
+ return this.http.get(url, options).pipe(
+ map(function (response) {
+ // export the VersionedFlowSnapshot by creating a hidden anchor element
+ var stringSnapshot = encodeURIComponent(JSON.stringify(response.body, null, 2));
Review comment:
Is it necessary to call `JSON.stringify()` or is it possible to just get the text content of response.body? If `response.body` is already JSON, that means it went through an unnecessary serialization step, which would be great to avoid if possible.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r631410991
##########
File path: nifi-registry-core/nifi-registry-web-ui/src/main/webapp/components/explorer/grid-list/dialogs/download-versioned-flow/nf-registry-download-versioned-flow.js
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { Component } from '@angular/core';
+import NfRegistryApi from 'services/nf-registry.api';
+import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material';
+import { FdsSnackBarService } from '@nifi-fds/core';
+
+/**
+ * NfRegistryDownloadVersionedFlow constructor.
+ *
+ * @param nfRegistryApi The api service.
+ * @param fdsSnackBarService The FDS snack bar service module.
+ * @param matDialogRef The angular material dialog ref.
+ * @param data The data passed into this component.
+ * @constructor
+ */
+function NfRegistryDownloadVersionedFlow(nfRegistryApi, fdsSnackBarService, matDialogRef, data) {
+ // Services
+ this.snackBarService = fdsSnackBarService;
+ this.nfRegistryApi = nfRegistryApi;
+ this.dialogRef = matDialogRef;
+ // local state
+ this.keepDialogOpen = false;
+ this.protocol = location.protocol;
+ this.droplet = data.droplet;
+ this.selectedVersion = this.droplet.snapshotMetadata[0].version;
+}
+
+NfRegistryDownloadVersionedFlow.prototype = {
+ constructor: NfRegistryDownloadVersionedFlow,
+
+ /**
+ * Download specified versioned flow snapshot.
+ */
+ downloadVersion: function () {
+ var self = this;
+ var version = this.selectedVersion;
+
+ this.nfRegistryApi.exportDropletVersionedSnapshot(this.droplet.link.href, version).subscribe(function (response) {
+ if (!response.status || response.status === 200) {
Review comment:
@exceptionfactory A successful response has typically been just the response body returned. So, when the body is returned (a success) there will not be a status property. I can see how this could be confusing at first glance and without much context. This if-statement is used throughout the codebase, so I decided to follow the conventions.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] andrewmlim commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
andrewmlim commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-841409765
> Recent changes include:
>
> * Changed the dialog title from 'Download Version' to 'Export Version' and refactored the component. @bbende @andrewmlim @moranr Thanks for the feedback and agreed 'Export' seems more consistent
> * Changed the 'Export Version' dialog dropdown menu item to "Latest (Version #)"
> * Changed the Actions menu hover style to differentiate from the focus style
These look good! Thanks for making all of these improvements @mtien-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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r631404406
##########
File path: nifi-registry-core/nifi-registry-web-ui/src/main/webapp/services/nf-registry.api.js
##########
@@ -75,6 +75,137 @@ NfRegistryApi.prototype = {
);
},
+ /**
+ * Retrieves the specified versioned flow snapshot for an existing droplet the registry has stored.
+ *
+ * @param {string} dropletUri The uri of the droplet to request.
+ * @param {number} versionNumber The version of the flow to request.
+ * @returns {*}
+ */
+ exportDropletVersionedSnapshot: function (dropletUri, versionNumber) {
+ var self = this;
+ var url = '../nifi-registry-api/' + dropletUri + '/versions/' + versionNumber + '/export';
+ var options = {
+ headers: headers,
+ observe: 'response'
+ };
+
+ return this.http.get(url, options).pipe(
+ map(function (response) {
+ // export the VersionedFlowSnapshot by creating a hidden anchor element
+ var stringSnapshot = encodeURIComponent(JSON.stringify(response.body, null, 2));
Review comment:
@exceptionfactory Good catch! I can return a `responseType: 'text'` and omit calling `JSON.stringify()`. The file content will not be in pretty print format but that seems ok to me.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r617188294
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +303,129 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Upload flow version",
+ notes = "Uploads the next version of a flow. The version number of the object being created must be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) })
+ public Response importVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier")
+ final String bucketId,
+ @PathParam("flowId")
+ @ApiParam(value = "The flow identifier")
+ final String flowId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("comments") final String comments) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ // deserialize InputStream to a VersionedFlowSnapshot
+ VersionedFlowSnapshot versionedFlowSnapshot;
+
+ versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ // clear or set the necessary snapShotMetadata
+ if (versionedFlowSnapshot.getSnapshotMetadata() != null) {
+ versionedFlowSnapshot.getSnapshotMetadata().setBucketIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setFlowIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setLink(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ // if there are new comments, then set it
+ // otherwise, keep the original comments
+ if (!StringUtils.isBlank(comments)) {
+ versionedFlowSnapshot.getSnapshotMetadata().setComments(comments);
+ }
+ }
+
+ return createFlowVersion(bucketId, flowId, versionedFlowSnapshot);
+ }
+
+ @POST
+ @Path("import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Create flow",
+ notes = "Creates a flow in the given bucket. The flow id is created by the server and populated in the returned entity.",
+ response = VersionedFlow.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}")})
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409)})
+ public Response importFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier") final String bucketId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("name") final String name,
+ @FormDataParam("description") final String description) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(name)) {
+ throw new IllegalArgumentException("The flow name is required.");
+ }
+
+ // create VersionedFlow
+ final VersionedFlow versionedFlow = new VersionedFlow();
+
+ versionedFlow.setName(name);
+ versionedFlow.setRevision(new RevisionInfo(null, 0L));
+ if (!StringUtils.isBlank(description)) {
+ versionedFlow.setDescription(description);
+ }
+
+ final VersionedFlow createdFlow = createAndPublishVersionedFlow(bucketId, versionedFlow);
+
+ // deserialize InputStream and create new VersionedFlowSnapshot
+ final VersionedFlowSnapshot versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ setSnaphotMetadataIfMissing(bucketId, createdFlow.getIdentifier(), versionedFlowSnapshot);
+
+ // set remaining snapshot metadata
+ final String userIdentity = NiFiUserUtils.getNiFiUserIdentity();
+ versionedFlowSnapshot.getSnapshotMetadata().setAuthor(userIdentity);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+
+ final VersionedFlowSnapshot createdSnapshot = serviceFacade.createFlowSnapshot(versionedFlowSnapshot);
+ publish(EventFactory.flowVersionCreated(createdSnapshot));
+
+ return Response.status(Response.Status.OK).entity(createdSnapshot).build();
Review comment:
This endpoint has been refactored and no longer returns a response 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] andrewmlim commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
andrewmlim commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-825757987
This new functionality is great! It is going to be extremely useful.
Here are some thoughts after some UI/UX testing:
- I think it would be an improvement if the "Delete data flow" action simply said "Delete flow". We refer to that type of asset in the Registry as "flow" and the documentation also refers to it as a "flow". The dialog that appears after selecting the action also says "Delete flow" so this change would help with consistency.
- The title for the "Delete flow" dialog should be changed to "Delete Flow" for case consistency with the "Import New Version" and "Download Version" dialogs.
- When a flow version is downloaded the default name of the file is always flow-version-#.json (where # is the version #). I think it would be helpful if the name would default to the flow name. So if I had a flow named "Kafka2HDFS" and wanted to download version 3 of it, the default name would be "Kafka2HDFS-version-3.json".
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r617193232
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +303,129 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Upload flow version",
+ notes = "Uploads the next version of a flow. The version number of the object being created must be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) })
+ public Response importVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier")
+ final String bucketId,
+ @PathParam("flowId")
+ @ApiParam(value = "The flow identifier")
+ final String flowId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("comments") final String comments) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ // deserialize InputStream to a VersionedFlowSnapshot
+ VersionedFlowSnapshot versionedFlowSnapshot;
+
+ versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ // clear or set the necessary snapShotMetadata
+ if (versionedFlowSnapshot.getSnapshotMetadata() != null) {
+ versionedFlowSnapshot.getSnapshotMetadata().setBucketIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setFlowIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setLink(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ // if there are new comments, then set it
+ // otherwise, keep the original comments
+ if (!StringUtils.isBlank(comments)) {
+ versionedFlowSnapshot.getSnapshotMetadata().setComments(comments);
+ }
+ }
+
+ return createFlowVersion(bucketId, flowId, versionedFlowSnapshot);
+ }
+
+ @POST
+ @Path("import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Create flow",
+ notes = "Creates a flow in the given bucket. The flow id is created by the server and populated in the returned entity.",
+ response = VersionedFlow.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}")})
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409)})
+ public Response importFlow(
Review comment:
My thought for creating this endpoint was to mainly handle the uploaded file and clear the snapshot metadata. It also made it a bit easier for the frontend to only make one request to the server. I have refactored some logic here to minimize duplicating code. It now returns `createFlowVersion` endpoint.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-822703531
This PR is still WIP, but is open for initial review. I still need to add integration tests and resolve some styling issues, specifically with text alignment in the dialogs.
@exceptionfactory @thenatog @bbende @pvillard31 Would you please review?
@scottyaslan @sardell @akocsis @andras Would you please review the frontend code?
Thank you!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r617189610
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +303,129 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Upload flow version",
+ notes = "Uploads the next version of a flow. The version number of the object being created must be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) })
+ public Response importVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier")
+ final String bucketId,
+ @PathParam("flowId")
+ @ApiParam(value = "The flow identifier")
+ final String flowId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("comments") final String comments) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ // deserialize InputStream to a VersionedFlowSnapshot
+ VersionedFlowSnapshot versionedFlowSnapshot;
+
+ versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ // clear or set the necessary snapShotMetadata
+ if (versionedFlowSnapshot.getSnapshotMetadata() != null) {
+ versionedFlowSnapshot.getSnapshotMetadata().setBucketIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setFlowIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setLink(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ // if there are new comments, then set it
+ // otherwise, keep the original comments
+ if (!StringUtils.isBlank(comments)) {
+ versionedFlowSnapshot.getSnapshotMetadata().setComments(comments);
+ }
+ }
+
+ return createFlowVersion(bucketId, flowId, versionedFlowSnapshot);
+ }
+
+ @POST
+ @Path("import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Create flow",
+ notes = "Creates a flow in the given bucket. The flow id is created by the server and populated in the returned entity.",
+ response = VersionedFlow.class,
Review comment:
Changed to VersionedFlowSnapshot.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r622688442
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +303,129 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Upload flow version",
+ notes = "Uploads the next version of a flow. The version number of the object being created must be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) })
+ public Response importVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier")
+ final String bucketId,
+ @PathParam("flowId")
+ @ApiParam(value = "The flow identifier")
+ final String flowId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("comments") final String comments) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ // deserialize InputStream to a VersionedFlowSnapshot
+ VersionedFlowSnapshot versionedFlowSnapshot;
+
+ versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ // clear or set the necessary snapShotMetadata
+ if (versionedFlowSnapshot.getSnapshotMetadata() != null) {
+ versionedFlowSnapshot.getSnapshotMetadata().setBucketIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setFlowIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setLink(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ // if there are new comments, then set it
+ // otherwise, keep the original comments
+ if (!StringUtils.isBlank(comments)) {
+ versionedFlowSnapshot.getSnapshotMetadata().setComments(comments);
+ }
+ }
+
+ return createFlowVersion(bucketId, flowId, versionedFlowSnapshot);
+ }
+
+ @POST
+ @Path("import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Create flow",
+ notes = "Creates a flow in the given bucket. The flow id is created by the server and populated in the returned entity.",
+ response = VersionedFlow.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}")})
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409)})
+ public Response importFlow(
Review comment:
@bbende I went ahead and removed the `importFlow` method. Now when importing a new flow, a call will first be made to `createFlow`, then another to `importVersionedFlow`.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] andrewmlim edited a comment on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
andrewmlim edited a comment on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-825757987
This new functionality is great! It is going to be extremely useful.
Here are some thoughts after some UI/UX testing:
- I think it would be an improvement if the "Delete data flow" action simply said "Delete flow". We refer to that type of asset in the Registry as "flow" and the documentation also refers to it as a "flow". The dialog that appears after selecting the action also says "Delete flow" so this change would help with consistency.
- The title for the "Delete flow" dialog should be changed to "Delete Flow" for case consistency with the "Import New Version" and "Download Version" dialogs.
- When a flow version is downloaded the default name of the file is always flow-version-#.json (where # is the version #). I think it would be helpful if the name would default to the flow name. So if I had a flow named "Kafka2HDFS" and wanted to download version 3 of it, the default name would be "Kafka2HDFS-version-3.json".
- I noticed that the color of the "DELETE" button in the "Delete Flow" dialog is a different color (more red) than the other dialog buttons (IMPORT and DOWNLOAD)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] moranr commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
moranr commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-826853222
Hey @mtien-apache - thanks for all your work on this. @andrewmlim 's comments all make sense to me and I would proceed with his recommendations. RE the Delete button color, yes the vibrant red is intentional as Drew observed. I believe this color is also used in other "delete" scenarios, so perhaps double-check to make sure color usage is consistent.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-840183039
Pushed commits that addresses all remaining changes and PR feedback. The PR is ready for a full review.
@sardell Would you review the frontend changes?
Recent changes include:
- Changed the dialog title from 'Download Version' to 'Export Version' and refactored the component. @bbende @andrewmlim @moranr Thanks for the feedback and agreed 'Export' seems more consistent
- Changed the 'Export Version' dialog dropdown menu item to "Latest (Version #)"
- Changed the Actions menu hover style to differentiate from the focus style
- Added frontend tests. Note - With the way Angular and Javascript is currently configured in Registry, I had issues writing certain tests for the new components. It's been captured in a JIRA ticket: [NIFIREG-454](https://issues.apache.org/jira/browse/NIFIREG-454). Once resolved, more tests can be added.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] andrewmlim commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
andrewmlim commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-830255068
I've seen the change where the name of the flow download is no longer always flow-version-#.json. But the name of the download defaults to the name of the process group defined in the flow definition not the name of the flow as I suggested in my earlier comments. For example, I import a new flow and give it the flow name "Kafka2HDFS" and select a flow definition file that has the name "ASDF" for the process group. This new flow is imported successfully. Within the NiFi Registry UI, there is nothing that references/displays "ASDF".
But if I choose to download this flow version from Registry, the name of the file is "ASDF-version-1.json". I expected it to be "Kafka2HDFS-version-1.json".
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r632768175
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +294,42 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.APPLICATION_JSON)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Import flow version",
+ notes = "Import the next version of a flow. The version number of the object being created will be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) })
+ public Response importVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier")
+ final String bucketId,
+ @PathParam("flowId")
+ @ApiParam(value = "The flow identifier")
+ final String flowId,
+ @ApiParam("file") final VersionedFlowSnapshot versionedFlowSnapshot,
+ @HeaderParam("Comments") final String comments) {
+
+ final VersionedFlowSnapshot createdSnapshot = serviceFacade.importVersionedFlowSnapshot(versionedFlowSnapshot, bucketId, flowId, comments);
+ publish(EventFactory.flowVersionCreated(createdSnapshot));
+ return Response.status(Response.Status.CREATED).entity(createdSnapshot).build();
Review comment:
@exceptionfactory I've added the header. Thanks!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] exceptionfactory commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r616786093
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -385,6 +520,64 @@ public Response getLatestFlowVersionMetadata(
return Response.status(Response.Status.OK).entity(latest).build();
}
+ @GET
+ @Path("{flowId}/versions/{versionNumber: \\d+}/export")
+ @Consumes(MediaType.WILDCARD)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Exports specified bucket flow version content",
+ notes = "Exports the specified version of a flow, including the metadata and content of the flow.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "read"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}")})
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409)})
+ public Response exportVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier") final String bucketId,
+ @PathParam("flowId")
+ @ApiParam("The flow identifier") final String flowId,
+ @PathParam("versionNumber")
+ @ApiParam("The version number") final Integer versionNumber) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ if (versionNumber == null) {
+ throw new IllegalArgumentException("The version number is required.");
+ }
+
+ final VersionedFlowSnapshot versionedFlowSnapshot = serviceFacade.getFlowSnapshot(bucketId, flowId, versionNumber);
+
+ versionedFlowSnapshot.setFlow(null);
+ versionedFlowSnapshot.setBucket(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setBucketIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setFlowIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setLink(null);
+
+ String attachmentName = "flow-version-" + versionNumber;
+
+ UriUtils.encodePath(attachmentName, StandardCharsets.UTF_8);
+
+ String filename = attachmentName + ".json";
+
+ final String versionedFlowSnapshotJsonString = serializeToJson(versionedFlowSnapshot);
+
+ return generateOkResponse(versionedFlowSnapshotJsonString).header(HttpHeaders.CONTENT_DISPOSITION, String.format("attachment; filename=\"%s\"", filename)).build();
Review comment:
For ease for reading, what do you think about formatting the content disposition separately and including the filename in the format?
```suggestion
final String versionedFlowSnapshotJsonString = serializeToJson(versionedFlowSnapshot);
final String contentDisposition = String.format("attachment; filename=\"file-version-%d.json\"", versionNumber)
return generateOkResponse(versionedFlowSnapshotJsonString).header(HttpHeaders.CONTENT_DISPOSITION, contentDisposition).build();
```
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -385,6 +520,64 @@ public Response getLatestFlowVersionMetadata(
return Response.status(Response.Status.OK).entity(latest).build();
}
+ @GET
+ @Path("{flowId}/versions/{versionNumber: \\d+}/export")
Review comment:
Is there supposed to be a space between `versionNumber` and the `\\d+` pattern?
##########
File path: nifi-registry-core/nifi-registry-web-ui/src/main/webapp/services/nf-registry.api.js
##########
@@ -75,6 +75,113 @@ NfRegistryApi.prototype = {
);
},
+ /**
+ * Retrieves the specified versioned flow snapshot for an existing droplet the registry has stored.
+ *
+ * @param {string} dropletUri The uri of the droplet to request.
+ * @param {number} versionNumber The version of the flow to request.
+ * @returns {*}
+ */
+ exportDropletVersionedSnapshot: function (dropletUri, versionNumber) {
+ var self = this;
+ var url = '../nifi-registry-api/' + dropletUri;
+ url += '/versions/' + versionNumber + '/export';
Review comment:
Is there a reason for building the `url` in two lines as opposed to one?
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +303,129 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Upload flow version",
+ notes = "Uploads the next version of a flow. The version number of the object being created must be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) })
+ public Response importVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier")
+ final String bucketId,
+ @PathParam("flowId")
+ @ApiParam(value = "The flow identifier")
+ final String flowId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("comments") final String comments) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ // deserialize InputStream to a VersionedFlowSnapshot
+ VersionedFlowSnapshot versionedFlowSnapshot;
+
+ versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ // clear or set the necessary snapShotMetadata
+ if (versionedFlowSnapshot.getSnapshotMetadata() != null) {
+ versionedFlowSnapshot.getSnapshotMetadata().setBucketIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setFlowIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setLink(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ // if there are new comments, then set it
+ // otherwise, keep the original comments
+ if (!StringUtils.isBlank(comments)) {
+ versionedFlowSnapshot.getSnapshotMetadata().setComments(comments);
+ }
+ }
+
+ return createFlowVersion(bucketId, flowId, versionedFlowSnapshot);
+ }
+
+ @POST
+ @Path("import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Create flow",
+ notes = "Creates a flow in the given bucket. The flow id is created by the server and populated in the returned entity.",
+ response = VersionedFlow.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}")})
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409)})
+ public Response importFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier") final String bucketId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("name") final String name,
+ @FormDataParam("description") final String description) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(name)) {
+ throw new IllegalArgumentException("The flow name is required.");
+ }
+
+ // create VersionedFlow
+ final VersionedFlow versionedFlow = new VersionedFlow();
+
+ versionedFlow.setName(name);
+ versionedFlow.setRevision(new RevisionInfo(null, 0L));
+ if (!StringUtils.isBlank(description)) {
+ versionedFlow.setDescription(description);
+ }
+
+ final VersionedFlow createdFlow = createAndPublishVersionedFlow(bucketId, versionedFlow);
+
+ // deserialize InputStream and create new VersionedFlowSnapshot
+ final VersionedFlowSnapshot versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ setSnaphotMetadataIfMissing(bucketId, createdFlow.getIdentifier(), versionedFlowSnapshot);
+
+ // set remaining snapshot metadata
+ final String userIdentity = NiFiUserUtils.getNiFiUserIdentity();
+ versionedFlowSnapshot.getSnapshotMetadata().setAuthor(userIdentity);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
Review comment:
Recommend updating this version reference as well with a static variable.
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +303,129 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Upload flow version",
+ notes = "Uploads the next version of a flow. The version number of the object being created must be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) })
+ public Response importVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier")
+ final String bucketId,
+ @PathParam("flowId")
+ @ApiParam(value = "The flow identifier")
+ final String flowId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("comments") final String comments) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ // deserialize InputStream to a VersionedFlowSnapshot
+ VersionedFlowSnapshot versionedFlowSnapshot;
+
+ versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ // clear or set the necessary snapShotMetadata
+ if (versionedFlowSnapshot.getSnapshotMetadata() != null) {
+ versionedFlowSnapshot.getSnapshotMetadata().setBucketIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setFlowIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setLink(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ // if there are new comments, then set it
+ // otherwise, keep the original comments
+ if (!StringUtils.isBlank(comments)) {
+ versionedFlowSnapshot.getSnapshotMetadata().setComments(comments);
+ }
+ }
+
+ return createFlowVersion(bucketId, flowId, versionedFlowSnapshot);
+ }
+
+ @POST
+ @Path("import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Create flow",
+ notes = "Creates a flow in the given bucket. The flow id is created by the server and populated in the returned entity.",
+ response = VersionedFlow.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}")})
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409)})
+ public Response importFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier") final String bucketId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("name") final String name,
+ @FormDataParam("description") final String description) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(name)) {
+ throw new IllegalArgumentException("The flow name is required.");
+ }
+
+ // create VersionedFlow
+ final VersionedFlow versionedFlow = new VersionedFlow();
+
+ versionedFlow.setName(name);
+ versionedFlow.setRevision(new RevisionInfo(null, 0L));
+ if (!StringUtils.isBlank(description)) {
+ versionedFlow.setDescription(description);
+ }
+
+ final VersionedFlow createdFlow = createAndPublishVersionedFlow(bucketId, versionedFlow);
+
+ // deserialize InputStream and create new VersionedFlowSnapshot
+ final VersionedFlowSnapshot versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ setSnaphotMetadataIfMissing(bucketId, createdFlow.getIdentifier(), versionedFlowSnapshot);
+
+ // set remaining snapshot metadata
+ final String userIdentity = NiFiUserUtils.getNiFiUserIdentity();
+ versionedFlowSnapshot.getSnapshotMetadata().setAuthor(userIdentity);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+
+ final VersionedFlowSnapshot createdSnapshot = serviceFacade.createFlowSnapshot(versionedFlowSnapshot);
+ publish(EventFactory.flowVersionCreated(createdSnapshot));
+
+ return Response.status(Response.Status.OK).entity(createdSnapshot).build();
Review comment:
Although Response.Status.OK is accurate, Response.Status.CREATED seems more applicable given that a resource is being created.
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -385,6 +520,64 @@ public Response getLatestFlowVersionMetadata(
return Response.status(Response.Status.OK).entity(latest).build();
}
+ @GET
+ @Path("{flowId}/versions/{versionNumber: \\d+}/export")
+ @Consumes(MediaType.WILDCARD)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Exports specified bucket flow version content",
+ notes = "Exports the specified version of a flow, including the metadata and content of the flow.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "read"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}")})
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409)})
+ public Response exportVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier") final String bucketId,
+ @PathParam("flowId")
+ @ApiParam("The flow identifier") final String flowId,
+ @PathParam("versionNumber")
+ @ApiParam("The version number") final Integer versionNumber) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ if (versionNumber == null) {
+ throw new IllegalArgumentException("The version number is required.");
+ }
+
+ final VersionedFlowSnapshot versionedFlowSnapshot = serviceFacade.getFlowSnapshot(bucketId, flowId, versionNumber);
+
+ versionedFlowSnapshot.setFlow(null);
+ versionedFlowSnapshot.setBucket(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setBucketIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setFlowIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setLink(null);
+
+ String attachmentName = "flow-version-" + versionNumber;
+
+ UriUtils.encodePath(attachmentName, StandardCharsets.UTF_8);
Review comment:
This line does not assign a variable, and it does not seem necessary since the attachmentName should not include any characters that require encoding based on the value assigned.
##########
File path: nifi-registry-core/nifi-registry-web-api/pom.xml
##########
@@ -423,6 +423,16 @@
<artifactId>jjwt</artifactId>
<version>0.7.0</version>
</dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-databind</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.protobuf</groupId>
+ <artifactId>protobuf-java</artifactId>
+ <version>3.11.4</version>
+ <scope>compile</scope>
+ </dependency>
Review comment:
Is this dependency necessary? There do not appear to be any references to protobuf in the changed classes.
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +303,129 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Upload flow version",
+ notes = "Uploads the next version of a flow. The version number of the object being created must be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) })
+ public Response importVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier")
+ final String bucketId,
+ @PathParam("flowId")
+ @ApiParam(value = "The flow identifier")
+ final String flowId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("comments") final String comments) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ // deserialize InputStream to a VersionedFlowSnapshot
+ VersionedFlowSnapshot versionedFlowSnapshot;
+
+ versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ // clear or set the necessary snapShotMetadata
+ if (versionedFlowSnapshot.getSnapshotMetadata() != null) {
+ versionedFlowSnapshot.getSnapshotMetadata().setBucketIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setFlowIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setLink(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
Review comment:
Since the value of `-1` is being used in multiple places, it would be helpful to define and reuse a static variable named something like `DEFAULT_VERSION` or `INITIAL_VERSION`.
##########
File path: nifi-registry-core/nifi-registry-web-ui/src/main/webapp/components/explorer/grid-list/dialogs/download-versioned-flow/nf-registry-download-versioned-flow.js
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { Component } from '@angular/core';
+import NfRegistryApi from 'services/nf-registry.api';
+import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material';
+import { FdsSnackBarService } from '@nifi-fds/core';
+
+/**
+ * NfRegistryDownloadVersionedFlow constructor.
+ *
+ * @param nfRegistryApi The api service.
+ * @param fdsSnackBarService The FDS snack bar service module.
+ * @param matDialogRef The angular material dialog ref.
+ * @param data The data passed into this component.
+ * @constructor
+ */
+function NfRegistryDownloadVersionedFlow(nfRegistryApi, fdsSnackBarService, matDialogRef, data) {
+ // Services
+ this.snackBarService = fdsSnackBarService;
+ this.nfRegistryApi = nfRegistryApi;
+ this.dialogRef = matDialogRef;
+ // local state
+ this.keepDialogOpen = false;
+ this.protocol = location.protocol;
+ this.droplet = data.droplet;
+ this.selectedVersion = this.droplet.snapshotMetadata[0].version;
+}
+
+NfRegistryDownloadVersionedFlow.prototype = {
+ constructor: NfRegistryDownloadVersionedFlow,
+
+ /**
+ * Download specified versioned flow snapshot.
+ */
+ downloadVersion: function () {
+ var self = this;
+ var version = this.selectedVersion;
+
+ this.nfRegistryApi.exportDropletVersionedSnapshot(this.droplet.link.href, version).subscribe(function (response) {
+ if (!response.status || response.status === 200) {
Review comment:
Does a false for `response.status` also indicate a success? Not quite following the logic.
##########
File path: nifi-registry-core/nifi-registry-web-ui/src/main/webapp/components/explorer/grid-list/dialogs/import-new-flow/nf-registry-import-new-flow.js
##########
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { Component } from '@angular/core';
+
+import NfRegistryApi from 'services/nf-registry.api';
+import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material';
+import { FdsSnackBarService } from '@nifi-fds/core';
+
+/**
+ * NfRegistryImportNewFlow constructor.
+ *
+ * @param nfRegistryApi The api service.
+ * @param fdsSnackBarService The FDS snack bar service module.
+ * @param matDialogRef The angular material dialog ref.
+ * @param data The data passed into this component.
+ * @constructor
+ */
+function NfRegistryImportNewFlow(nfRegistryApi, fdsSnackBarService, matDialogRef, data) {
+ // Services
+ this.snackBarService = fdsSnackBarService;
+ this.nfRegistryApi = nfRegistryApi;
+ this.dialogRef = matDialogRef;
+ // local state
+ this.keepDialogOpen = false;
+ this.protocol = location.protocol;
+ this.buckets = data.buckets;
+ this.writableBuckets = [];
+ this.fileToUpload = null;
+ this.fileName = null;
+ this.name = '';
+ this.description = '';
+ this.selectedBucket = '';
+ this.hoverValidity = '';
+ this.extensions = 'application/json';
+ this.multiple = false;
+}
+
+NfRegistryImportNewFlow.prototype = {
+ constructor: NfRegistryImportNewFlow,
+
+ ngOnInit: function () {
+ // only show buckets the user can write
+ var self = this;
+ self.writableBuckets = self.filterWritableBuckets(self.buckets);
+ },
+
+ //TODO: get from the service instead
Review comment:
Is the goal to implement this filtering as part of the PR? It could be useful as a convenience methods or optional parameter, but the client-side filtering seems acceptable given that the buckets are visible to the user, just available with different permissions.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] moranr commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
moranr commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-838476301
> I think the most consistent label would be "Latest (Version #)".
@mtien-apache I like that suggestion.
> "Export Version" is a good suggestion. Like Bryan, I don't think that change is critical.
Also agree here, but not critical. Typically you see Upload/Download or Import/Export used in together.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-832217308
> In the Actions drop-down, "Import new version" is always highlighted even when another selection has the focus.
>
> 
@andrewmlim (cc: @moranr ) It seems like the initial focus behavior is based on keyboard actions and from an accessibility perspective, it's the expected behavior. When using the arrow keys, the focus moves with the new selection. I think what is making it confusing is that the keyboard and mouseover selections have the same red highlighting styles. I believe this highlighting behavior was always present but probably wasn't an issue since there had only been 1 menu item (Delete).
One proposed fix is I can dig in to change the styles between a keyboard selection and mouseover selection, something like in other dropdown menus in Registry:
<img width="400" alt="Screen Shot 2021-05-04 at 11 29 44 AM" src="https://user-images.githubusercontent.com/56892372/117064443-111f2080-acdb-11eb-86c9-0c26df6031bf.png">
Thoughts?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] andrewmlim edited a comment on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
andrewmlim edited a comment on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-825757987
This new functionality is great! It is going to be extremely useful.
Here are some thoughts after some UI/UX testing:
- I think it would be an improvement if the "Delete data flow" action simply said "Delete flow". We refer to that type of asset in the Registry as "flow" and the documentation also refers to it as a "flow". The dialog that appears after selecting the action also says "Delete flow" so this change would help with consistency.
- The title for the "Delete flow" dialog should be changed to "Delete Flow" for case consistency with the "Import New Version" and "Download Version" dialogs.
- When a flow version is downloaded the default name of the file is always flow-version-#.json (where # is the version #). I think it would be helpful if the name would default to the flow name. So if I had a flow named "Kafka2HDFS" and wanted to download version 3 of it, the default name would be "Kafka2HDFS-version-3.json".
- I noticed that the color of the "DELETE" button in the "Delete Flow" dialog is a different color (more red) than the other dialog buttons (IMPORT and DOWNLOAD). This appears to be intentional. The "DELETE" button for deleting a bucket is the same red, so understandable if the color difference is to make it stand out since it is a unrecoverable action.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] andrewmlim commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
andrewmlim commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-830256771
In the Actions drop-down, "Import new version" is always highlighted even when another selection has the focus.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-832204208
Pushed a commit:
- Added a new `ExportedVersionedFlowSnapshot` object
- Fixed Import New Version dialog to show the next version to be imported
- Refactored frontend and server side code.
- Added frontend and backend integration tests.
Will add more frontend tests.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] andrewmlim commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
andrewmlim commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-837401042
> One proposed fix is I can dig in to change the styles between a keyboard selection and mouseover selection, something like in other dropdown menus in Registry:
>
> <img alt="Screen Shot 2021-05-04 at 11 29 44 AM" width="400" src="https://user-images.githubusercontent.com/56892372/117064443-111f2080-acdb-11eb-86c9-0c26df6031bf.png">
>
> Thoughts?
That would be nice if you could change the styles. That would definitely help distinguish the two difference selections.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r617188914
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -385,6 +520,64 @@ public Response getLatestFlowVersionMetadata(
return Response.status(Response.Status.OK).entity(latest).build();
}
+ @GET
+ @Path("{flowId}/versions/{versionNumber: \\d+}/export")
+ @Consumes(MediaType.WILDCARD)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Exports specified bucket flow version content",
+ notes = "Exports the specified version of a flow, including the metadata and content of the flow.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "read"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}")})
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409)})
+ public Response exportVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier") final String bucketId,
+ @PathParam("flowId")
+ @ApiParam("The flow identifier") final String flowId,
+ @PathParam("versionNumber")
+ @ApiParam("The version number") final Integer versionNumber) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ if (versionNumber == null) {
+ throw new IllegalArgumentException("The version number is required.");
+ }
+
+ final VersionedFlowSnapshot versionedFlowSnapshot = serviceFacade.getFlowSnapshot(bucketId, flowId, versionNumber);
+
+ versionedFlowSnapshot.setFlow(null);
+ versionedFlowSnapshot.setBucket(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setBucketIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setFlowIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setLink(null);
+
+ String attachmentName = "flow-version-" + versionNumber;
+
+ UriUtils.encodePath(attachmentName, StandardCharsets.UTF_8);
Review comment:
I've removed the encoding.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r617188040
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -385,6 +520,64 @@ public Response getLatestFlowVersionMetadata(
return Response.status(Response.Status.OK).entity(latest).build();
}
+ @GET
+ @Path("{flowId}/versions/{versionNumber: \\d+}/export")
+ @Consumes(MediaType.WILDCARD)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Exports specified bucket flow version content",
+ notes = "Exports the specified version of a flow, including the metadata and content of the flow.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "read"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}")})
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409)})
+ public Response exportVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier") final String bucketId,
+ @PathParam("flowId")
+ @ApiParam("The flow identifier") final String flowId,
+ @PathParam("versionNumber")
+ @ApiParam("The version number") final Integer versionNumber) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ if (versionNumber == null) {
+ throw new IllegalArgumentException("The version number is required.");
+ }
+
+ final VersionedFlowSnapshot versionedFlowSnapshot = serviceFacade.getFlowSnapshot(bucketId, flowId, versionNumber);
+
+ versionedFlowSnapshot.setFlow(null);
+ versionedFlowSnapshot.setBucket(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setBucketIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setFlowIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setLink(null);
+
+ String attachmentName = "flow-version-" + versionNumber;
+
+ UriUtils.encodePath(attachmentName, StandardCharsets.UTF_8);
+
+ String filename = attachmentName + ".json";
+
+ final String versionedFlowSnapshotJsonString = serializeToJson(versionedFlowSnapshot);
+
+ return generateOkResponse(versionedFlowSnapshotJsonString).header(HttpHeaders.CONTENT_DISPOSITION, String.format("attachment; filename=\"%s\"", filename)).build();
Review comment:
Agreed that's easier to read. I've made the suggested changes. Thanks!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] sardell commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
sardell commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-841502608
@mtien-apache I just finished looking at the UI code and testing your changes in the UI. Nice work. I'm a +1 (non-binding).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] bbende commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
bbende commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-836748695
Latest changes look good!
One minor comment... when downloading a version, the window that pops up has the latest version labeled as "Latest Version (v.#)" but then the other items in the list are just "Version #", seems a little inconsistent that one has "v." and the others don't. Should the latest one just say "Latest Version (#)" or maybe "Version # (latest)" ?
Also, while writing this, I started thinking about how we have generally been referring to this as import/export, and the buttons for import use the word "Import", but the for export we are calling it "Download Version". I'm not too hung up on this, but started wondering if that should be called "Export Version" to better correspond with "Import"?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] exceptionfactory commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r632501622
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +294,42 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.APPLICATION_JSON)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Import flow version",
+ notes = "Import the next version of a flow. The version number of the object being created will be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) })
+ public Response importVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier")
+ final String bucketId,
+ @PathParam("flowId")
+ @ApiParam(value = "The flow identifier")
+ final String flowId,
+ @ApiParam("file") final VersionedFlowSnapshot versionedFlowSnapshot,
+ @HeaderParam("Comments") final String comments) {
+
+ final VersionedFlowSnapshot createdSnapshot = serviceFacade.importVersionedFlowSnapshot(versionedFlowSnapshot, bucketId, flowId, comments);
+ publish(EventFactory.flowVersionCreated(createdSnapshot));
+ return Response.status(Response.Status.CREATED).entity(createdSnapshot).build();
Review comment:
Best practice when returning an HTTP 201 includes adding a `Location` header with the URL of the new resource. See `ApplicationResource.generateCreatedResponse()` as a helper method for returning the Location header with the created entity.
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +294,42 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.APPLICATION_JSON)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Import flow version",
+ notes = "Import the next version of a flow. The version number of the object being created will be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
Review comment:
Adding an `ApiResponse` annotation with a code of `201` would be helpful for completeness in the generated documentation.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] andrewmlim commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
andrewmlim commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-830221602
Testing the latest changes and noticed that whenever I select "Import new version", the version is always shown as "v.2" in the "Import New Version" dialog even if is it the 3rd, 4th, 5th, etc. version that I am importing.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r617195342
##########
File path: nifi-registry-core/nifi-registry-web-ui/src/main/webapp/components/explorer/grid-list/dialogs/import-new-flow/nf-registry-import-new-flow.js
##########
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { Component } from '@angular/core';
+
+import NfRegistryApi from 'services/nf-registry.api';
+import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material';
+import { FdsSnackBarService } from '@nifi-fds/core';
+
+/**
+ * NfRegistryImportNewFlow constructor.
+ *
+ * @param nfRegistryApi The api service.
+ * @param fdsSnackBarService The FDS snack bar service module.
+ * @param matDialogRef The angular material dialog ref.
+ * @param data The data passed into this component.
+ * @constructor
+ */
+function NfRegistryImportNewFlow(nfRegistryApi, fdsSnackBarService, matDialogRef, data) {
+ // Services
+ this.snackBarService = fdsSnackBarService;
+ this.nfRegistryApi = nfRegistryApi;
+ this.dialogRef = matDialogRef;
+ // local state
+ this.keepDialogOpen = false;
+ this.protocol = location.protocol;
+ this.buckets = data.buckets;
+ this.writableBuckets = [];
+ this.fileToUpload = null;
+ this.fileName = null;
+ this.name = '';
+ this.description = '';
+ this.selectedBucket = '';
+ this.hoverValidity = '';
+ this.extensions = 'application/json';
+ this.multiple = false;
+}
+
+NfRegistryImportNewFlow.prototype = {
+ constructor: NfRegistryImportNewFlow,
+
+ ngOnInit: function () {
+ // only show buckets the user can write
+ var self = this;
+ self.writableBuckets = self.filterWritableBuckets(self.buckets);
+ },
+
+ //TODO: get from the service instead
Review comment:
My goal is to have this be like a utility method in Angular service (`nf-registry.service.js`). It would only show the buckets the user is authorized to write. I've been having issues injecting it into this import-new-flow component, so I'm in the middle of troubleshooting that.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-827030107
> Hey @mtien-apache - thanks for all your work on this. @andrewmlim 's comments all make sense to me and I would proceed with his recommendations. RE the Delete button color, yes the vibrant red is intentional as Drew observed. I believe this color is also used in other "delete" scenarios, so perhaps double-check to make sure color usage is consistent.
@moranr Thanks for confirming - will proceed with the UI changes. @andrewmlim Confirmed the Delete button color is the same as other Delete buttons used throughout Registry (eg, Delete Buckets).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r617186563
##########
File path: nifi-registry-core/nifi-registry-web-api/pom.xml
##########
@@ -423,6 +423,16 @@
<artifactId>jjwt</artifactId>
<version>0.7.0</version>
</dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-databind</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.protobuf</groupId>
+ <artifactId>protobuf-java</artifactId>
Review comment:
Removed - this was accidentally leftover from some previous 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] bbende commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
bbende commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r616802214
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +303,129 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Upload flow version",
+ notes = "Uploads the next version of a flow. The version number of the object being created must be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) })
+ public Response importVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier")
+ final String bucketId,
+ @PathParam("flowId")
+ @ApiParam(value = "The flow identifier")
+ final String flowId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("comments") final String comments) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ // deserialize InputStream to a VersionedFlowSnapshot
+ VersionedFlowSnapshot versionedFlowSnapshot;
+
+ versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ // clear or set the necessary snapShotMetadata
+ if (versionedFlowSnapshot.getSnapshotMetadata() != null) {
+ versionedFlowSnapshot.getSnapshotMetadata().setBucketIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setFlowIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setLink(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ // if there are new comments, then set it
+ // otherwise, keep the original comments
+ if (!StringUtils.isBlank(comments)) {
+ versionedFlowSnapshot.getSnapshotMetadata().setComments(comments);
+ }
+ }
+
+ return createFlowVersion(bucketId, flowId, versionedFlowSnapshot);
+ }
+
+ @POST
+ @Path("import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Create flow",
+ notes = "Creates a flow in the given bucket. The flow id is created by the server and populated in the returned entity.",
+ response = VersionedFlow.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}")})
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409)})
+ public Response importFlow(
Review comment:
I was wondering if we really need this end-point. Since there is already an existing POST to `/buckets/{bucketId}/flows` for creating a flow, could the front-end call that first to create the flow and then call the import version end-point? This is would make it similar to how it works when NiFi first starts version control on a PG, it calls create flow then adds the first snapshot.
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +303,129 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Upload flow version",
+ notes = "Uploads the next version of a flow. The version number of the object being created must be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) })
+ public Response importVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier")
+ final String bucketId,
+ @PathParam("flowId")
+ @ApiParam(value = "The flow identifier")
+ final String flowId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("comments") final String comments) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ // deserialize InputStream to a VersionedFlowSnapshot
+ VersionedFlowSnapshot versionedFlowSnapshot;
+
+ versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ // clear or set the necessary snapShotMetadata
+ if (versionedFlowSnapshot.getSnapshotMetadata() != null) {
+ versionedFlowSnapshot.getSnapshotMetadata().setBucketIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setFlowIdentifier(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setLink(null);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ versionedFlowSnapshot.getSnapshotMetadata().setVersion(-1);
+ // if there are new comments, then set it
+ // otherwise, keep the original comments
+ if (!StringUtils.isBlank(comments)) {
+ versionedFlowSnapshot.getSnapshotMetadata().setComments(comments);
+ }
+ }
+
+ return createFlowVersion(bucketId, flowId, versionedFlowSnapshot);
+ }
+
+ @POST
+ @Path("import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Create flow",
+ notes = "Creates a flow in the given bucket. The flow id is created by the server and populated in the returned entity.",
+ response = VersionedFlow.class,
Review comment:
If we keep this end-point, then this should be VersionedFlowSnapshot based on what the method is returning below.
##########
File path: nifi-registry-core/nifi-registry-web-api/pom.xml
##########
@@ -423,6 +423,16 @@
<artifactId>jjwt</artifactId>
<version>0.7.0</version>
</dependency>
+ <dependency>
+ <groupId>com.fasterxml.jackson.core</groupId>
+ <artifactId>jackson-databind</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.protobuf</groupId>
+ <artifactId>protobuf-java</artifactId>
Review comment:
I haven't made it through all the code yet, but what did we need protocol buffers for?
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +303,129 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Upload flow version",
+ notes = "Uploads the next version of a flow. The version number of the object being created must be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) })
+ public Response importVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier")
+ final String bucketId,
+ @PathParam("flowId")
+ @ApiParam(value = "The flow identifier")
+ final String flowId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("comments") final String comments) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ // deserialize InputStream to a VersionedFlowSnapshot
+ VersionedFlowSnapshot versionedFlowSnapshot;
+
+ versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ // clear or set the necessary snapShotMetadata
+ if (versionedFlowSnapshot.getSnapshotMetadata() != null) {
Review comment:
I think we always want to clear these metadata values for the case where someone has json from another registry that was obtained before your new export existed. We probably also need to protect against the metadata being null because I think it is null coming from "Download Flow" in NiFi, so maybe always set to a new metadata and then apply anything on top of that?
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -385,6 +520,64 @@ public Response getLatestFlowVersionMetadata(
return Response.status(Response.Status.OK).entity(latest).build();
}
+ @GET
+ @Path("{flowId}/versions/{versionNumber: \\d+}/export")
+ @Consumes(MediaType.WILDCARD)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Exports specified bucket flow version content",
+ notes = "Exports the specified version of a flow, including the metadata and content of the flow.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "read"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}")})
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409)})
+ public Response exportVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier") final String bucketId,
+ @PathParam("flowId")
+ @ApiParam("The flow identifier") final String flowId,
+ @PathParam("versionNumber")
+ @ApiParam("The version number") final Integer versionNumber) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ if (versionNumber == null) {
+ throw new IllegalArgumentException("The version number is required.");
+ }
+
+ final VersionedFlowSnapshot versionedFlowSnapshot = serviceFacade.getFlowSnapshot(bucketId, flowId, versionNumber);
Review comment:
This is kind of minor, but it might be nice to move most of this logic behind the service facade to a new method like exportFlowSnapshot(bucketId, flowId, versionNumber). We tried to make it so most of the logic happens behind the service facade, and the REST end-points don't do too much.
There is also an example of an existing file-download for extension bundles, which uses the StreamingContent/StreamingOutput, it is not as big a deal for this case since the JSON probably won't ever be too large, but in case you want to try and use this approach...
https://github.com/apache/nifi-registry/blob/main/nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/ExtensionRepoResource.java#L437-L445
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-828895145
Pushed 2 commits:
- Handled the scenario when an invalid snapshot file is uploaded during Import New Flow @andrewmlim
- Refactored client side API import methods
- In the upload file methods, I removed `formData` and now passing the file as an API param. Also using HTTP headers to pass additional data
- In `uploadFlow`, I'm now making multiple calls to the server that re-uses the existing `BucketFlowResource.createFlow` method
- Refactored server side import methods
- Removed `BucketFlowResource.importFlow`
- Refactored logic from `BucketFlowResource.importVersionedFlow` to the service facade
UI changes:
- Fixed remaining UI style issues
- In the 'Import New Flow' dialog, the initial value for the Bucket dropdown menu is set to the Bucket the user is currently viewing. Otherwise, no value is set when viewing all buckets.
- Added messages to inform the user why the 'Import New Flow' button is displayed
- One displays a message in the center of the page when there are no existing buckets
- Another message is displayed under the button buckets exist but the user does not have any write permissions
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache commented on a change in pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache commented on a change in pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#discussion_r617187216
##########
File path: nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java
##########
@@ -291,6 +303,129 @@ public Response createFlowVersion(
return Response.status(Response.Status.OK).entity(createdSnapshot).build();
}
+ @POST
+ @Path("{flowId}/versions/import")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(
+ value = "Upload flow version",
+ notes = "Uploads the next version of a flow. The version number of the object being created must be the " +
+ "next available version integer. Flow versions are immutable after they are created.",
+ response = VersionedFlowSnapshot.class,
+ extensions = {
+ @Extension(name = "access-policy", properties = {
+ @ExtensionProperty(name = "action", value = "write"),
+ @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") })
+ }
+ )
+ @ApiResponses({
+ @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400),
+ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
+ @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403),
+ @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404),
+ @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) })
+ public Response importVersionedFlow(
+ @PathParam("bucketId")
+ @ApiParam("The bucket identifier")
+ final String bucketId,
+ @PathParam("flowId")
+ @ApiParam(value = "The flow identifier")
+ final String flowId,
+ @FormDataParam("file") final InputStream in,
+ @FormDataParam("comments") final String comments) {
+
+ if (StringUtils.isBlank(bucketId)) {
+ throw new IllegalArgumentException("The bucket identifier is required.");
+ }
+
+ if (StringUtils.isBlank(flowId)) {
+ throw new IllegalArgumentException("The flow identifier is required.");
+ }
+
+ // deserialize InputStream to a VersionedFlowSnapshot
+ VersionedFlowSnapshot versionedFlowSnapshot;
+
+ versionedFlowSnapshot = deserializeVersionedFlowSnapshot(in);
+
+ // clear or set the necessary snapShotMetadata
+ if (versionedFlowSnapshot.getSnapshotMetadata() != null) {
Review comment:
Good idea to set new metadata first - I revised this to create a new snapshotMetadata then set any necessary properties from there.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] andrewmlim commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
andrewmlim commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-825819989
Testing how the feature does error handling and noticed the following:
When importing new flow, if I select a JSON file that is valid JSON, but not a valid flow definition file, after selecting import I get the error message "Cannot create versioned flow snapshot - flowContents must not be null". After closing the dialog, it seems like nothing was imported, but if you refresh the view a new flow is imported with version 0. (As an aside, if you select "Download version" on this corrupt flow, the UI seems to open a dialog on the left side of the window.)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] andrewmlim commented on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
andrewmlim commented on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-837401773
I tested a secure Registry with different authorizations given to users (on the users themselves and also on bucket policies). Auth looks good!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-registry] mtien-apache edited a comment on pull request #319: NIFIREG-395 - Implemented the ability to import and export versioned flows through the UI
Posted by GitBox <gi...@apache.org>.
mtien-apache edited a comment on pull request #319:
URL: https://github.com/apache/nifi-registry/pull/319#issuecomment-828895145
Pushed 2 commits:
- Handled the scenario when an invalid snapshot file is uploaded during Import New Flow @andrewmlim
- Refactored client side API import methods
- In the upload file methods, I removed `formData` and now passing the file as an API param. Also using HTTP headers to pass additional data
- In `uploadFlow`, I'm now making multiple calls to the server that re-uses the existing `BucketFlowResource.createFlow` method
- Refactored server side import methods
- Removed `BucketFlowResource.importFlow`
- Refactored logic from `BucketFlowResource.importVersionedFlow` to the service facade
UI changes:
- Fixed remaining UI style issues
- In the 'Import New Flow' dialog, the initial value for the Bucket dropdown menu is set to the Bucket the user is currently viewing. Otherwise, no value is set when viewing all buckets.
- Added messages to inform the user why the 'Import New Flow' button is displayed
- One displays a message in the center of the page when there are no existing buckets
- Another message is displayed under the button when buckets exist but the user does not have any write permissions
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org