Add Data store as resource on backend #2385

danielbdias · 2023-04-13T23:59:42Z

This PR adds the Data Store as a resource into the Tracetest server.

On this PR, both the old and new structures coexist on the API. The next PR will update the OpenAPI code with make generate and change the references to use this structure.

Loom: https://www.loom.com/share/45b49110030d4b0688b51c059f97388c

Changes

Add DataStore resource package
Updated internal references to use this package
Removed DataStore model

Fixes

[CLI Improvements] Migrate DataStores to resource manager architecture - Backend #2341
(partially) [CLI Improvements] Migrate DataStores to resource manager architecture #2334

Checklist

tested locally
added new dependencies
updated the docs
added a test

danielbdias · 2023-04-14T13:15:00Z

server/tracedb/datastoreresource/repository.go

Right now, I've added datastoreresource package inside of tracedb, because I thought they manage the same thing, even knowing that tracedb is heavily infrastructure-based code.

I accept suggestions on other places that could not be confusing 🙃

I think this makes sense. I was thinking that maybe we don't need a package specifically for the resource. Maybe it could live directly under tracedb? This is an open question

we discussed this and decided we'll keep the separated project. this approach allows more flexibility

schoren · 2023-04-17T13:34:12Z

server/tracedb/datastoreresource/_tests.go

this looks like a weird name. is it like this for any particular reason?

An ill-succeed experiment that I've done to try to focus all repository tests on the same place. 🤦 🙃
On later versions, I've changed to repository_tests.go

schoren · 2023-04-17T14:02:38Z

server/tracedb/datastoreresource/repository.go

+
+	// reuse the created_at field for auditing purposes,
+	// unless the client explicitly send it
+	createdAt, err := r.getCreatedAt(ctx, dataStore)


do we need to support this case? I think unless we have an explicit need for this, clients should not be allowed to set this

We have this case today, I believe that we can remove it on the future.

schoren · 2023-04-17T14:06:10Z

server/model/data_stores.go

@@ -111,6 +111,7 @@ func (ds DataStore) HasID() bool {
 }

 func (ds DataStore) Validate() error {
+	//TODO: replace for resource validate


would it make sense to address these TODOs in this PR?

Yes! I'm working on it.

…2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural

…esource-on-backend

danielbdias · 2023-04-20T20:30:21Z

server/tracedb/datastoreresource/repository_test.go

+	return manager
+}
+
+func cleanup(t *testing.T, manager resourcemanager.Manager) {


Without a cleanup structure, this test file surpasses more than 100 idle connections. Later we need to think about how we can improve the RM structure to avoid these open connections.

…esource-on-backend

jorgeepc

LGTM!

… CLI commands (#2414) * Updating DataStore API definition to consider ResourcesAPI structure (#2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural * Add Data store as resource on Backend (#2385) * feat(frontend): add data store as resource (#2423) * Add data store as resource on CLI (#2422) * Updating DataStore API definition to consider ResourcesAPI structure (#2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural * Adding datastoreresource package * Update internal references to use datastoreresource package * Isolate database resources on each test * Adding behavior for no datastore scenario * Fixing rebase problems * Fixing another rebase problem * Updating DataStore API definition to consider ResourcesAPI structure (#2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural * Fixing merge * Adding documentation for delete operation * Improving cleanup code * Updating datastore API * Adding CLI generated code * wip * wip * Fixing CLI * deprecated old commands * Adding wrongly deleted file * simplifying the resource actions creation steps --------- Co-authored-by: Oscar Reyes <oscar-rreyes1@hotmail.com> * fix ingester dsRepo * chore(docs): update data store docs and examples (#2429) * removing test deletion * fixing provisioning file * fixing provisioning file --------- Co-authored-by: Jorge Padilla <jorge.esteban.padilla@gmail.com> Co-authored-by: Oscar Reyes <oscar-rreyes1@hotmail.com>

danielbdias force-pushed the add/data-store-as-resource-on-backend branch 2 times, most recently from bf2e9ec to 7e859f5 Compare April 14, 2023 00:13

danielbdias changed the title ~~Add/data store as resource on backend~~ Add Data store as resource on backend Apr 14, 2023

danielbdias marked this pull request as ready for review April 14, 2023 12:56

danielbdias commented Apr 14, 2023

View reviewed changes

danielbdias requested review from mathnogueira, jorgeepc and schoren April 14, 2023 16:28

danielbdias force-pushed the add/data-store-as-resource-on-backend branch 3 times, most recently from de55b1f to 7ace777 Compare April 14, 2023 20:31

danielbdias mentioned this pull request Apr 14, 2023

Updating DataStore API definition to consider ResourcesAPI structure #2368

Merged

4 tasks

schoren reviewed Apr 17, 2023

View reviewed changes

danielbdias force-pushed the add/data-store-as-resource-on-backend branch 2 times, most recently from df280fd to a0b165a Compare April 17, 2023 21:13

danielbdias force-pushed the feature/datastore-as-resource branch from 17add7e to 17fdb8f Compare April 18, 2023 01:21

danielbdias added the do not merge label Apr 18, 2023

danielbdias force-pushed the add/data-store-as-resource-on-backend branch 3 times, most recently from ab5b1c4 to ed03a5f Compare April 19, 2023 18:32

danielbdias mentioned this pull request Apr 19, 2023

Updating tests to isolate database resources and connections per module #2409

Merged

4 tasks

Updating DataStore API definition to consider ResourcesAPI structure (#…

a997d88

…2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural

danielbdias force-pushed the feature/datastore-as-resource branch from 17fdb8f to a997d88 Compare April 19, 2023 21:08

danielbdias added 4 commits April 19, 2023 18:09

Adding datastoreresource package

337149d

Update internal references to use datastoreresource package

96823ce

Isolate database resources on each test

0af8e9c

Adding behavior for no datastore scenario

156958c

danielbdias force-pushed the add/data-store-as-resource-on-backend branch from 1b524b2 to 156958c Compare April 19, 2023 21:18

Fixing rebase problems

60574ad

danielbdias removed the do not merge label Apr 20, 2023

danielbdias and others added 2 commits April 20, 2023 10:58

Fixing another rebase problem

02c43f2

Updating DataStore API definition to consider ResourcesAPI structure (#…

ea05beb

…2368) Fixing sorting fields on OpenAPI Updating OpenAPI to consider DataStore endpoint managing only one record Add endpoint in plural

danielbdias force-pushed the feature/datastore-as-resource branch from a997d88 to ea05beb Compare April 20, 2023 18:59

danielbdias added 2 commits April 20, 2023 16:58

Merge branch 'feature/datastore-as-resource' into add/data-store-as-r…

3fa9212

…esource-on-backend

Fixing merge

dbb2f2a

danielbdias commented Apr 20, 2023

View reviewed changes

danielbdias added 3 commits April 20, 2023 20:52

Adding documentation for delete operation

77b73b0

Merge branch 'main' into add/data-store-as-resource-on-backend

dc318a3

Improving cleanup code

6a1808f

danielbdias force-pushed the add/data-store-as-resource-on-backend branch from b4b9ee8 to 6a1808f Compare April 21, 2023 14:09

Updating datastore API

0edfd9a

danielbdias mentioned this pull request Apr 21, 2023

Add data store as resource on CLI #2422

Merged

4 tasks

danielbdias force-pushed the feature/datastore-as-resource branch from ea05beb to 57eb095 Compare April 21, 2023 19:53

danielbdias added 2 commits April 21, 2023 16:54

Merge branch 'feature/datastore-as-resource' into add/data-store-as-r…

dc482fd

…esource-on-backend

Fixing problems with /config/connection

667e8aa

danielbdias requested a review from xoscar April 24, 2023 13:40

jorgeepc approved these changes Apr 24, 2023

View reviewed changes

danielbdias merged commit a3eafe2 into feature/datastore-as-resource Apr 24, 2023
18 of 27 checks passed

danielbdias deleted the add/data-store-as-resource-on-backend branch April 24, 2023 17:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Data store as resource on backend #2385

Add Data store as resource on backend #2385

danielbdias commented Apr 13, 2023 •

edited

danielbdias Apr 14, 2023 •

edited

schoren Apr 17, 2023

schoren Apr 17, 2023

schoren Apr 17, 2023

danielbdias Apr 24, 2023 •

edited

schoren Apr 17, 2023

danielbdias Apr 24, 2023

schoren Apr 17, 2023

danielbdias Apr 17, 2023

danielbdias Apr 20, 2023

jorgeepc left a comment

Add Data store as resource on backend #2385

Add Data store as resource on backend #2385

Conversation

danielbdias commented Apr 13, 2023 • edited

Changes

Fixes

Checklist

danielbdias Apr 14, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielbdias Apr 24, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorgeepc left a comment

Choose a reason for hiding this comment

danielbdias commented Apr 13, 2023 •

edited

danielbdias Apr 14, 2023 •

edited

danielbdias Apr 24, 2023 •

edited