[#2181] Merge internalFilename and originalFilename #2430
Conversation
github-actions
bot
added
java
|
@muyangye thank you for updating the PR. Regarding the exception in the test, I guess you have to mock the static method somehow, otherwise it tries to connect to the couchdb. |
|
@tenthe I will try to see if I can mock the static method somehow. Thanks for pointing out! Did you leave some small comments on code? For some reason I did not see them? |
| private String filetype; | ||
|
|
||
| private long createdAt; | ||
| private long lastModified; | ||
|
|
||
| private String createdByUser; | ||
|
|
||
| public String test; |
There was a problem hiding this comment.
Can this variable be deleted?
There was a problem hiding this comment.
Yes, this is solely for testing purposes to see what will happen if I add another attribute to the FileMetadata class, will delete.
| deleteFile(oldFilename); | ||
| storeFile(newFilename, fileInputStream); | ||
| } catch (Exception e) { | ||
| System.out.println(e.getMessage()); |
There was a problem hiding this comment.
Please use a logger instead. Can we change the generic Exception with the specific exceptions that can be thrown?
There was a problem hiding this comment.
Both are definitely doable, I will push my changes together with the unit tests.
Sorry, they were still on pending. Can you now see them? |
|
@tenthe I have pushed an update that addressed your code review and added unit tests. I admit that the code is really ugly given that I added an |
|
Hello there 👋 |
github-actions
bot
added
the
stale
|
Hi @tenthe, did you have a chance to review my unit tests? |
github-actions
bot
removed
the
stale
ui/src/app/core-ui/static-properties/static-file-input/static-file-input.component.ts
Outdated
Show resolved
Hide resolved
...che/streampipes/service/core/migrations/v095/MergeFilenamesAndRenameDuplicatesMigration.java
Show resolved
Hide resolved
Co-authored-by: Tim <50115603+bossenti@users.noreply.github.com>
|
Thank you very much for your contribution here @muyangye 🙏🏼 |
Purpose
See #2181, finally merging internalFilename and originalFilename to one.
Remarks
Result:
I created 3 files with the same name using StreamPipes 0.93.0 release:
After which I run the latest migration which is successful:
However, for pipelines, specifically Data Lake (not adapters, adapters can produce messages as usual (i.e. if I create another pipeline using the same adapter created in v0.93.0, I can still visualize it correctly)) created before using File Stream, DataLake expects a new schema:
After which all pipelines are back to normal:
Method:
Get raw json stored in CouchDB => use file ID as the "link" between old version of
FileMetadataand new version ofFileMetadata=> then updateFileMetadataand File stored locally.Test:
I haven't written any unit tests yet because I don't know how to test it since
FileMetadataclass was changed. My original thought is this: create a new class calledoldFileMetadatawhich is the same as theFileMetadataclass that has both originalFilename and internalFilename => store that object using aCouchDbClientin the FileMetadata database => run the migration. However, I don't understand why butUtils.getCouchDbFileMetadataClient()throws an exception of "unknown host" in file I createdstreampipes-service-core/src/test/java/org.apache.streampipes.service.core.migrations.v095/MergeFilenamesAndRenameDuplicatesTest.javaso I deleted it. I believe it's a package/config issue, do you have any idea why? Happy to write a unit test after fixing this issue and everyone agrees with my approach :)PR introduces (a) breaking change(s): yes? Please let me know if the required manual schema update of Data Lake is a problem.
PR introduces (a) deprecation(s): no