Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

file by instance id sdk support #799

Open
wants to merge 43 commits into
base: master
Choose a base branch
from

Conversation

Jacob-Eliat-Eliat
Copy link
Contributor

@Jacob-Eliat-Eliat Jacob-Eliat-Eliat commented Nov 19, 2024

This should allow file content support to use instance id instead of just external Id.

Note: update is also supported by instance id, and will only allow updates on the fields which cannot be set using fdm. This is to be done in another PR. It is not necessary for file content support, but might be for file support.

Copy link

codecov bot commented Nov 19, 2024

Codecov Report

Attention: Patch coverage is 75.86207% with 14 lines in your changes missing coverage. Please review.

Project coverage is 84.67%. Comparing base (0d0546c) to head (2a41cd5).

Files with missing lines Patch % Lines
...ain/scala/com/cognite/sdk/scala/v1/dataTypes.scala 20.00% 8 Missing ⚠️
...om/cognite/sdk/scala/common/ReadableResource.scala 83.33% 3 Missing ⚠️
...ala/com/cognite/sdk/scala/v1/resources/files.scala 90.47% 2 Missing ⚠️
...n/scala/com/cognite/sdk/scala/common/package.scala 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #799      +/-   ##
==========================================
- Coverage   84.82%   84.67%   -0.16%     
==========================================
  Files          96       96              
  Lines        2676     2728      +52     
  Branches      204      189      -15     
==========================================
+ Hits         2270     2310      +40     
- Misses        406      418      +12     
Files with missing lines Coverage Δ
...scala/com/cognite/sdk/scala/common/dataTypes.scala 84.74% <ø> (ø)
...rc/main/scala/com/cognite/sdk/scala/v1/files.scala 100.00% <ø> (ø)
...n/scala/com/cognite/sdk/scala/common/package.scala 96.55% <88.88%> (-3.45%) ⬇️
...ala/com/cognite/sdk/scala/v1/resources/files.scala 92.30% <90.47%> (-0.75%) ⬇️
...om/cognite/sdk/scala/common/ReadableResource.scala 86.56% <83.33%> (-1.67%) ⬇️
...ain/scala/com/cognite/sdk/scala/v1/dataTypes.scala 33.33% <20.00%> (-6.67%) ⬇️

Copy link

github-actions bot commented Dec 1, 2024

This pull request seems a bit stale.. Shall we invite more to the party?

cogniteIds: Seq[CogniteIdOrInstanceId],
ignoreUnknownIds: Boolean
)(implicit itemsDecoder: Decoder[Items[R]]): F[Seq[R]] =
requestSession.post[Seq[R], Items[R], ItemsWithIgnoreUnknownIds[CogniteIdOrInstanceId]](
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
requestSession.post[Seq[R], Items[R], ItemsWithIgnoreUnknownIds[CogniteIdOrInstanceId]](
requestSession.post[Seq[R], Items[R], ItemsWithIgnoreUnknownIds[InstanceId]](

still had a not on this method being called byInstanceIds but taking it CogniteIdOrInstanceId, should either be renamed or changed to accept according to name

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants