Upsert #1287

doctrino · 2023-07-13T18:17:42Z

Description

Done ready for full review.

Found a issue with Sequence.columns added an issue on it here; #1292

But saving that implementation for later.

Checklist:

Tests added/updated.
Documentation updated. Documentation is generated from docstrings - these must be updated according to your change.
If a new method has been added it should be referenced in cognite.rst in order to generate docs based on its docstring.
Changelog updated in CHANGELOG.md.
Version bumped. If triggering a new release is desired, bump the version number in _version.py and pyproject.toml per semantic versioning.

codecov · 2023-07-13T18:23:05Z

Codecov Report

Merging #1287 (0ca71d1) into master (345f622) will decrease coverage by 0.06%.
The diff coverage is 86.78%.

❗ Current head 0ca71d1 differs from pull request most recent head d5fe468. Consider uploading reports for the commit d5fe468 to get more accurate results

@@            Coverage Diff             @@
##           master    #1287      +/-   ##
==========================================
- Coverage   90.75%   90.69%   -0.06%     
==========================================
  Files         104      104              
  Lines       12120    12269     +149     
==========================================
+ Hits        10999    11127     +128     
- Misses       1121     1142      +21

Impacted Files	Coverage Δ
cognite/client/_api/assets.py	`94.92% <77.77%> (-0.60%)`	⬇️
cognite/client/_api/events.py	`90.16% <77.77%> (-2.29%)`	⬇️
cognite/client/_api/relationships.py	`95.77% <77.77%> (-2.64%)`	⬇️
cognite/client/_api/time_series.py	`90.16% <77.77%> (-2.29%)`	⬇️
cognite/client/_api/sequences.py	`94.31% <78.57%> (-1.44%)`	⬇️
cognite/client/_api_client.py	`91.91% <80.64%> (-1.68%)`	⬇️
cognite/client/_api/annotations.py	`88.88% <100.00%> (ø)`
cognite/client/_version.py	`100.00% <100.00%> (ø)`
cognite/client/data_classes/_base.py	`94.35% <100.00%> (+0.13%)`	⬆️
cognite/client/data_classes/annotations.py	`96.66% <100.00%> (+0.11%)`	⬆️
... and 13 more

... and 2 files with indirect coverage changes

cognite/client/_api_client.py

cognite/client/_api/events.py

tests/tests_integration/test_api/test_events.py

cognite/client/_api_client.py

cognite/client/_api/events.py

haakonvt

This is good stuff, yo

cognite/client/_api/events.py

cognite/client/_api_client.py

cognite/client/_api/assets.py

cognite/client/_api/files.py

cognite/client/_api/time_series.py

cognite/client/_api_client.py

haakonvt · 2023-07-18T20:24:40Z

cognite/client/data_classes/assets.py

+            PropertySpec("name", is_nullable=False),
+            PropertySpec("description"),
+            PropertySpec("data_set_id"),
+            PropertySpec("metadata", is_list=True),


The is_list name feels odd. What about is_object or is_container?

I think is_object is misleading. Not a strong preference, although I lean towards staying is_list. @erlendvollset Thought?

is_list sounds wrong for objects yes. how about just is_primitive?

I think is_primitive would be the opposite of is_list? In that case I will go with is_container

cognite/client/data_classes/_base.py

cognite/client/data_classes/files.py

tests/tests_integration/test_api/test_events.py

erlendvollset

Let's add a section to the docs describing the patch vs. replace behaviour.

cognite/client/data_classes/_base.py

erlendvollset · 2023-07-19T11:11:30Z

cognite/client/data_classes/assets.py

+            PropertySpec("name", is_nullable=False),
+            PropertySpec("description"),
+            PropertySpec("data_set_id"),
+            PropertySpec("metadata", is_list=True),


is_list sounds wrong for objects yes. how about just is_primitive?

cognite/client/data_classes/assets.py

cognite/client/_api_client.py

erlendvollset · 2023-07-19T11:53:26Z

We should also make it very explicit that this upsert operation is not atomic.

…ent modes

erlendvollset

LGTM!

doctrino marked this pull request as ready for review July 16, 2023 10:21

doctrino requested review from a team as code owners July 16, 2023 10:21

haakonvt reviewed Jul 17, 2023

View reviewed changes

cognite/client/_api_client.py Show resolved Hide resolved

doctrino requested a review from haakonvt July 18, 2023 08:18

doctrino force-pushed the core-upsert branch from 47bcb2b to b41524b Compare July 18, 2023 11:28

erlendvollset reviewed Jul 18, 2023

View reviewed changes

cognite/client/_api/events.py Outdated Show resolved Hide resolved

doctrino force-pushed the core-upsert branch from b41524b to 65bf40d Compare July 18, 2023 16:07

doctrino requested a review from erlendvollset July 18, 2023 19:14

doctrino force-pushed the core-upsert branch from e6c17aa to 0c9e688 Compare July 18, 2023 19:15

haakonvt reviewed Jul 18, 2023

View reviewed changes

doctrino force-pushed the core-upsert branch from 85a9275 to 9c85bb7 Compare July 19, 2023 07:21

erlendvollset reviewed Jul 19, 2023

View reviewed changes

doctrino requested a review from erlendvollset July 19, 2023 14:40

doctrino added 13 commits July 19, 2023 17:10

refactor: Shell for upsert functionality

07ed7f3

tests: Added failing test

2514cc1

feat: Implemented upsert for event

3547e0b

fix: With failing test

e434267

fix: Only ID specified, with failing test

e04b3e2

fix: Handle multiple task split with failing test

7c0f4d7

tests: Added test with invalid events

a885fe9

refactor: Rewrote _convert_serource_to_patch_object to support differ…

72e2e00

…ent modes

feat: Upgraded the events.upsert to use mode

ff5e99a

refactor: Review feedback

bee8536

fix: no implicit optional

cd17b34

refactor: Moved tests

fb2931a

refactor: Introduce PropertySpec

dc54aa0

doctrino added 26 commits July 19, 2023 17:10

docs: Fix docs tests

06fd51f

docs: update docs

bc01980

build: bump

0012453

tests: assert multiple calls

6ef5845

docs: improved mode parameter documentation

de8ed68

refactor: review feedback"

7e396dd

refactor: review feedback 2, update forgotten Update classes

5993908

refactor: review feedback 3

f7d072f

tests: Updated test data

26cec7d

fix: invalid mode passet in (with failing test)

88f7ea7

fix: not found internal id

db32220

tests: Added a few more test for completeness

e93ed7a

fix: individual resource test with fixes

5accc07

fix: relationship test with fix

1912a0e

fix: bug in files update property spec

0cc41d6

build: changelog update

ace1e9c

tests: updated tests data

7b15560

refactor: remove files.upsert

e04f13e

build: bump changelog

1af603b

refactor: review round 3

c656ad1

refactor: renaming is_list to is_container

50cb166

tests: missing update spec

e94a105

docs: asset

f445445

docs: upsert in appendix

ee49d14

docs: ref appendix in upsert methods

99be266

build bump

d5fe468

doctrino force-pushed the core-upsert branch from 0ca71d1 to d5fe468 Compare July 19, 2023 15:11

erlendvollset approved these changes Jul 19, 2023

View reviewed changes

doctrino merged commit cdb8f88 into master Jul 19, 2023

doctrino deleted the core-upsert branch July 19, 2023 15:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upsert #1287

Upsert #1287

doctrino commented Jul 13, 2023 •

edited

Loading

codecov bot commented Jul 13, 2023 •

edited

Loading

haakonvt left a comment

haakonvt Jul 18, 2023

doctrino Jul 19, 2023

erlendvollset Jul 19, 2023

doctrino Jul 19, 2023

erlendvollset left a comment

erlendvollset Jul 19, 2023

erlendvollset commented Jul 19, 2023

erlendvollset left a comment

Upsert #1287

Upsert #1287

Conversation

doctrino commented Jul 13, 2023 • edited Loading

Description

Checklist:

codecov bot commented Jul 13, 2023 • edited Loading

Codecov Report

haakonvt left a comment

Choose a reason for hiding this comment

haakonvt Jul 18, 2023

Choose a reason for hiding this comment

doctrino Jul 19, 2023

Choose a reason for hiding this comment

erlendvollset Jul 19, 2023

Choose a reason for hiding this comment

doctrino Jul 19, 2023

Choose a reason for hiding this comment

erlendvollset left a comment

Choose a reason for hiding this comment

erlendvollset Jul 19, 2023

Choose a reason for hiding this comment

erlendvollset commented Jul 19, 2023

erlendvollset left a comment

Choose a reason for hiding this comment

doctrino commented Jul 13, 2023 •

edited

Loading

codecov bot commented Jul 13, 2023 •

edited

Loading