DataHub Connector #350
Merged
DataHub Connector #350
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Code Coverage
Files
|
hirenkp2000
requested changes
Sep 27, 2024
...http/src/main/java/com/here/naksha/storage/http/connector/ConnectorInterfaceReadExecute.java
Outdated
Show resolved
Hide resolved
...http/src/main/java/com/here/naksha/storage/http/connector/ConnectorInterfaceReadExecute.java
Outdated
Show resolved
Hide resolved
adamczyk-HERE
force-pushed
the
http_connector_storage
branch
from
8c71fb2 to
c25cc01
Compare
Code Coverage
Files
|
adamczyk-HERE
changed the title
Code Coverage
Files
|
adamczyk-HERE
force-pushed
the
http_connector_storage
branch
from
c25cc01 to
91cea2b
Compare
Code Coverage
Files
|
hirenkp2000
approved these changes
Oct 3, 2024
adamczyk-HERE
force-pushed
the
http_connector_storage
branch
from
81f2b2a to
02f6176
Compare
adamczyk-HERE
force-pushed
the
http_connector_storage
branch
from
02f6176 to
bf6e61b
Compare
Code Coverage
Files
|
adamczyk-HERE
force-pushed
the
http_connector_storage
branch
from
bf6e61b to
03b886b
Compare
Code Coverage
Files
|
Code Coverage
Files
|
Code Coverage
Files
|
Code Coverage
Files
|
adamczyk-HERE
force-pushed
the
http_connector_storage
branch
from
5e1f3ae to
cdb38cb
Compare
hirenkp2000
requested changes
Oct 30, 2024
Member
hirenkp2000
left a comment
hirenkp2000
left a comment
There was a problem hiding this comment.
Changes look good so far. Pls look into minor comments and also address pipeline failure.
here-naksha-storage-http/src/main/java/com/here/naksha/storage/http/PrepareResult.java
Outdated
Show resolved
Hide resolved
...http/src/main/java/com/here/naksha/storage/http/connector/ConnectorInterfaceReadExecute.java
Outdated
Show resolved
Hide resolved
...http/src/main/java/com/here/naksha/storage/http/connector/ConnectorInterfaceReadExecute.java
Outdated
Show resolved
Hide resolved
adamczyk-HERE
force-pushed
the
http_connector_storage
branch
3 times, most recently
from
e01e40f to
029d13a
Compare
Code Coverage
Files
|
Signed-off-by: Adamczyk, Tomasz <[email protected]> Signed-off-by: adamczyk-HERE <[email protected]>
Signed-off-by: adamczyk-HERE <[email protected]>
Signed-off-by: adamczyk-HERE <[email protected]>
Signed-off-by: adamczyk-HERE <[email protected]>
Signed-off-by: adamczyk-HERE <[email protected]>
Code Coverage
Files
|
hirenkp2000
requested changes
Nov 22, 2024
...ttp/src/main/java/com/here/naksha/storage/http/connector/ConnectorInterfaceWriteExecute.java
Outdated
Show resolved
Hide resolved
...ttp/src/main/java/com/here/naksha/storage/http/connector/ConnectorInterfaceWriteExecute.java
Outdated
Show resolved
Hide resolved
Signed-off-by: adamczyk-HERE <[email protected]>
adamczyk-HERE
force-pushed
the
http_connector_storage
branch
from
6619309 to
2ed8782
Compare
Code Coverage
Files
|
Code Coverage
Files
|
Signed-off-by: adamczyk-HERE <[email protected]>
Code Coverage
Files
|
Signed-off-by: adamczyk-HERE <[email protected]>
Signed-off-by: adamczyk-HERE <[email protected]>
adamczyk-HERE
force-pushed
the
http_connector_storage
branch
from
2e42dda to
6534eb7
Compare
Code Coverage
Files
|
Code Coverage
Files
|
Code Coverage
Files
|
Signed-off-by: adamczyk-HERE <[email protected]>
adamczyk-HERE
force-pushed
the
http_connector_storage
branch
from
2e8f750 to
005d16f
Compare
Code Coverage
Files
|
Signed-off-by: adamczyk-HERE <[email protected]>
Code Coverage
Files
|
hirenkp2000
requested changes
Dec 9, 2024
| List<XyzFeature> featuresToUpdate = new LinkedList<>(); | ||
| Map<String, String> featuresToDelete = new HashMap<>(); // Format enforced by connector API | ||
|
|
||
| populateDbCache(request.features); |
Member
There was a problem hiding this comment.
I assume we don't need to prefetch for DELETE ?
Contributor
Author
There was a problem hiding this comment.
We don't.
Modified code in populateDbCache method to get only features with PUT operation.
| String uuid = feature.getProperties().getXyzNamespace().getUuid(); | ||
| // We are making an assumption that if uuid exists in feature, it is not a new feature | ||
| return uuid == null && !existsInDb(feature); | ||
| } |
Member
There was a problem hiding this comment.
As we are anyways pre-fetching all records from DB, I think it is safer to update isNewFeature() function to only check whether feature exists in DB (i.e. in cache) or not. And avoid additional uuid == null condition. This will otherwise prevent those API requests where features are new and uuid is provided by user (either mistakenly OR purposely to fail request on UUID conflict)
Contributor
Author
There was a problem hiding this comment.
Done.
Also added throwing if a new feature with uuid is provided (assertNoUuid method)
This is how DataHub behaves and now as we dropped assumption that "uuid exists => its update operation" I thought it is good to implement.
| } | ||
| } | ||
|
|
||
| private void setPuuidFromUuid(XyzFeature feature) { |
Member
There was a problem hiding this comment.
to be safer, I suggest always setting puuid with the value of uuid as available in DB (and not with the uuid of the incoming request).
Contributor
Author
There was a problem hiding this comment.
Done and tested
| } | ||
| } | ||
|
|
||
| private void fillMissingCreatedAt(XyzFeature feature) { |
Member
There was a problem hiding this comment.
to be safer, I suggest always setting createdAt with the value of createdAt as available in DB (and not with the value supplied in the incoming request). My assumption here is, user should not be allowed to manipulate createdAt (intentionally or unintentionally).
in that case the function name can be renamed as something like fillOriginalCreatedAt()
Contributor
Author
There was a problem hiding this comment.
Done and tested
Signed-off-by: adamczyk-HERE <[email protected]>
Code Coverage
Files
|
Signed-off-by: adamczyk-HERE <[email protected]>
Signed-off-by: adamczyk-HERE <[email protected]>
Signed-off-by: adamczyk-HERE <[email protected]>
Code Coverage
Files
|
hirenkp2000
approved these changes
Jan 27, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.