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

Dont sent TypeNameMarker as type, convert to type-strings #908

Merged
merged 5 commits into from
Aug 27, 2014

Conversation

Tasteful
Copy link
Contributor

TypeNameMarker was converted as NEST.TypeNameMarger in the querystring when fetching the _client.IndicesStats. Convert the TypeNameMarker as the correct type with the help of ElasticInferrer.

@gmarz
Copy link
Contributor

gmarz commented Aug 26, 2014

@Tasteful great catch, thanks! Looks like this is failing one of the unit tests though: BigBadUrlUnitTests.TestAllTheUrls. Mind taking a look?

Also, small nit pick- can you change:

((IIndicesStatsRequest)this).Types = completion_fields; 

to

Self.Types = completion_fields;  

It's a pattern we've been following for convenience in all the descriptors so we don't have to continuously cast to the IRequest object.

@Tasteful
Copy link
Contributor Author

@gmarz I have now updated the code to follow the pattern and added some more unit tests. It was a easy NullPointerException for the failed unit test so that was easy fixed.

@Tasteful
Copy link
Contributor Author

@gmarz Maybe the parameter-name should be types instead of completion_fields in the IndicesStatsDescriptor.Types()?

@Mpdreamz
Copy link
Member

@Tasteful +1 on renaming the parameter to types.

@Tasteful
Copy link
Contributor Author

@gmarz Code updated.

@gmarz
Copy link
Contributor

gmarz commented Aug 26, 2014

@Tasteful yes, I think the completion_fields param should be renamed to types.

Also, I left a note on your last commit. I think the code you added to UpdatePathInfo is already being done in IndicesStatsPathInfo.Update(), which checks the Types collection on the request instance that's passed to it. I think the only fix that was required here was changing that line to Self.Types = completion_fields in the Types() descriptor method.

The reason for the static IndicesStatsPathInfo.Update() is so that we ensure the path info modifications are applied to both IndicesStatsRequest and IndicesStatsDescriptor in the same manner. You'll see this is the pattern we follow in all the Descriptor files.

@gmarz
Copy link
Contributor

gmarz commented Aug 27, 2014

@Tasteful LGTM now, thanks again for this!

gmarz added a commit that referenced this pull request Aug 27, 2014
Dont sent TypeNameMarker  as type, convert to type-strings
@gmarz gmarz merged commit a963df2 into elastic:develop Aug 27, 2014
@Tasteful Tasteful deleted the fix/indexstat branch August 27, 2014 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants