Skip to content

TSDB: Test create_doc permission#86638

Merged
nik9000 merged 2 commits intoelastic:masterfrom
nik9000:tsdb_create_doc_permission
May 11, 2022
Merged

TSDB: Test create_doc permission#86638
nik9000 merged 2 commits intoelastic:masterfrom
nik9000:tsdb_create_doc_permission

Conversation

@nik9000
Copy link
Copy Markdown
Member

@nik9000 nik9000 commented May 10, 2022

This adds a test for the create_doc and index permissions into tsdb
indices.

@nik9000 nik9000 added >test Issues or PRs that are addressing/adding tests :StorageEngine/TSDB You know, for Metrics v8.3.0 labels May 10, 2022
@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label May 10, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

This adds a test for the `create_doc` and `index` permissions into tsdb
indices.
@sethmlarson sethmlarson added the Team:Clients Meta label for clients team label May 10, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/clients-team (Team:Clients)

Copy link
Copy Markdown
Contributor

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for adding a test!

@nik9000
Copy link
Copy Markdown
Member Author

nik9000 commented May 10, 2022

@elasticmachine update branch

Copy link
Copy Markdown
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

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

The test LGTM, but I have a question.

index:
refresh: true
index: test
op_type: index
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm curious what happens without this?
I assume that AutoIdHandler will set the op_type to create and then TSDB will reject the request because a document with the (auto generated) id already exists.

Is that the intended semantics of the {ts-index}/_doc endpoint - if there's a possibility that there might be an existing doc for the same tsid, then you need to set the op_type or be prepared to handle a failure?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the AutoIdHandler will only set the op_type to create if no op_type has been specified. So it will overwrite a document?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah. It's a bit weird to me, but that's what happens. Without this you get create and overwrites stop in their tracks. I think it's a good thing that this isn't index dependent, but it is weird. And for _bulk we don't see it because folks say create or index. It's a quirk that'll have to be documented for tsdb.

@nik9000 nik9000 merged commit 3a87440 into elastic:master May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:StorageEngine/TSDB You know, for Metrics Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Clients Meta label for clients team >test Issues or PRs that are addressing/adding tests v8.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants