Skip to content
New issue

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

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

Already on GitHub? # to your account

[BUG] GetSourceRequest has unnecessary field _source #283

Closed
zethuman opened this issue Apr 6, 2023 · 8 comments · Fixed by #402
Closed

[BUG] GetSourceRequest has unnecessary field _source #283

zethuman opened this issue Apr 6, 2023 · 8 comments · Fixed by #402
Labels
bug Something isn't working

Comments

@zethuman
Copy link
Contributor

zethuman commented Apr 6, 2023

What is the bug?

The field _source is not needed, since this parameter has no functionality, since only the option is available - true

How can one reproduce the bug?

Input:

r: opensearchapi.GetSourceRequest{
  Index:      index,
  DocumentID: "2",
  Source:     []string{"false"},
}

Output:
{[{action_request_validation_exception Validation Failed: 1: fetching source can not be disabled; }] action_request_validation_exception Validation Failed: 1: fetching source can not be disabled; }

What is the expected behavior?

Default option is true logically there must be option with false. Expected to does not include the _source field in the response body.

What is your host/environment?

MacOS Ventura / opensearchproject/opensearch:latest / Go 1.15

Possible solution

I can remove this one, since it is not logical to be able to pass a negative value in a request whose purpose is to receive this data.

@zethuman zethuman added bug Something isn't working untriaged labels Apr 6, 2023
@wbeckler wbeckler removed the untriaged label Apr 7, 2023
@wbeckler
Copy link

wbeckler commented Apr 7, 2023

Could fixing this break someone's working code? If so, we might want to put this change into the 3.x line.

@zethuman
Copy link
Contributor Author

zethuman commented Apr 7, 2023

Yes, it may affect the end user code. So, we can already commit into the 3.x line?

@wbeckler
Copy link

wbeckler commented Apr 7, 2023

Is there a way to fix this that preserves working code?

@zethuman
Copy link
Contributor Author

zethuman commented Apr 7, 2023

I think not, since we will remove the source field from the structure, those clients who already have this field in the structure (even if it does not carry functionality) will receive an syntax error. Therefore, it is better to plan for future versions.

@VachaShah
Copy link
Collaborator

Can we deprecate this parameter in the next version and then remove in the next major version?

@zethuman
Copy link
Contributor Author

Yes, why not. If we fix it in this version it will break everything, it's best to plan for the next version

@VachaShah
Copy link
Collaborator

@zethuman I was looking at this and looks like the _source field can be used to provide list of fields to be returned. https://github.com/opensearch-project/opensearch-go/blob/main/opensearchapi/api.get_source.go#L239. I tried it in my sample code and it works:

getSourceRequest := opensearchapi.GetSourceRequest{
		Index:      IndexName,
		DocumentID: "1",
		Source:     []string{"title"},
	}

The response was [200 OK] {"title":"Moneyball"}

So the code needs to be fixed rather than _source parameter removed entirely. I will raise a PR soon for this.

@VachaShah
Copy link
Collaborator

It is however similar to SourceIncludes then, so we can deprecate one parameter, and moving forward remove in future major versions.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants