Wrapping results in Hashie::Mash 3.5.1 causes logs to be flooded with warnings #398

Closed
lparry opened this Issue Feb 1, 2017 · 9 comments

Comments

Projects
None yet
5 participants
@lparry

lparry commented Feb 1, 2017

I updated our bundle today which included hashie@3.5.1, and found our logs are now completely overwhelmed with warnings:

WARN -- : You are setting a key that conflicts with a built-in method Hashie::Mash#sort defined in Enumerable. This can cause unexpected behavior when accessing the key via as a property. You can still access the key via the #[] method.

I believe this is being caused by the "sort" key that comes back in the response body. eg: "sort":[1.121931762,1477653971450]. If I remove any use of sort from the query the warnings go away.

we're doing our searching via elasticsearch/persistence/repository if that has any bearing on things

I suspect it's related to this PR: intridea/hashie#381. I don't have a good suggestion how it might be fixed other than to stop using Hashie, but that seems like a pretty large change

@bmclean

This comment has been minimized.

Show comment
Hide comment
@bmclean

bmclean Feb 1, 2017

@lparry There is a discussion on hashie issue #391 as well.

bmclean commented Feb 1, 2017

@lparry There is a discussion on hashie issue #391 as well.

@gfx

This comment has been minimized.

Show comment
Hide comment
@gfx

gfx Feb 6, 2017

It also occurs on my project.

Why elasticsearch-ruby uses hashie? Sometimes it breaks projects.

gfx commented Feb 6, 2017

It also occurs on my project.

Why elasticsearch-ruby uses hashie? Sometimes it breaks projects.

@karmi

This comment has been minimized.

Show comment
Hide comment
@karmi

karmi Feb 6, 2017

Member

Hi guys, thanks for the heads-up!

I'll try it locally, and see what could be done about it. I'm afraid nothing much, since we cannot avoid the sort property...

(...) we're doing our searching via elasticsearch/persistence/repository if that has any bearing on things (...)

Yes, it does! :), since the error is actually not coming from gems in this repository (the Ruby client or the DSL extension), but from the Rails integration repository. It would be great if you could open an issue there, referencing this one, but no worries if not, I can do it myself.

Member

karmi commented Feb 6, 2017

Hi guys, thanks for the heads-up!

I'll try it locally, and see what could be done about it. I'm afraid nothing much, since we cannot avoid the sort property...

(...) we're doing our searching via elasticsearch/persistence/repository if that has any bearing on things (...)

Yes, it does! :), since the error is actually not coming from gems in this repository (the Ruby client or the DSL extension), but from the Rails integration repository. It would be great if you could open an issue there, referencing this one, but no worries if not, I can do it myself.

@karmi

This comment has been minimized.

Show comment
Hide comment
@karmi

karmi Feb 6, 2017

Member

Why elasticsearch-ruby uses hashie? Sometimes it breaks projects.

@gfx, I was thinking that the elasticsearch-dsl extension uses it, but no, this is coming from the Rails integration I've linked above. Hashie::Mash is used there because that's arguable the most easy way of having a "dot notation access" (results.hits.hits.first.person.first_name) to the JSON returned by Elasticsearch... Suggestions for a different way of handling that are welcome!

Member

karmi commented Feb 6, 2017

Why elasticsearch-ruby uses hashie? Sometimes it breaks projects.

@gfx, I was thinking that the elasticsearch-dsl extension uses it, but no, this is coming from the Rails integration I've linked above. Hashie::Mash is used there because that's arguable the most easy way of having a "dot notation access" (results.hits.hits.first.person.first_name) to the JSON returned by Elasticsearch... Suggestions for a different way of handling that are welcome!

@lparry

This comment has been minimized.

Show comment
Hide comment
@lparry

lparry Feb 6, 2017

Have opened the same issue on the correct repo, will leave this open to point people there rather than opening it here again

lparry commented Feb 6, 2017

Have opened the same issue on the correct repo, will leave this open to point people there rather than opening it here again

@gfx

This comment has been minimized.

Show comment
Hide comment
@gfx

gfx Feb 7, 2017

@karmi

I think Struct is the alternative. Fortunately, some IDEs, e.g. RubyMine, can introspect the definition of Struct and shows us code completion. Really useful.

gfx commented Feb 7, 2017

@karmi

I think Struct is the alternative. Fortunately, some IDEs, e.g. RubyMine, can introspect the definition of Struct and shows us code completion. Really useful.

@karmi

This comment has been minimized.

Show comment
Hide comment
@karmi

karmi Feb 8, 2017

Member

@gfx, that wouldn't work, unfortunately, please see my reply at elastic/elasticsearch-rails#666.

Member

karmi commented Feb 8, 2017

@gfx, that wouldn't work, unfortunately, please see my reply at elastic/elasticsearch-rails#666.

@michaelherold

This comment has been minimized.

Show comment
Hide comment
@michaelherold

michaelherold Feb 10, 2017

I just released Hashie v3.5.2 which has the disable_warnings feature for the Mash logger and sets the Hashie logger to the Rails logger when we detect that we're running in Rails. Hopefully, this helps!

I just released Hashie v3.5.2 which has the disable_warnings feature for the Mash logger and sets the Hashie logger to the Rails logger when we detect that we're running in Rails. Hopefully, this helps!

@karmi

This comment has been minimized.

Show comment
Hide comment
@karmi

karmi Feb 12, 2017

Member

A HashWrapper subclass has been added elastic/elasticsearch-rails@5732fa3, and used as the wrapper across the elasticsearch-model and elasticsearch-persistence gems, so the logs/STDERR should be quiet now.


Closed via elastic/elasticsearch-rails#666

Member

karmi commented Feb 12, 2017

A HashWrapper subclass has been added elastic/elasticsearch-rails@5732fa3, and used as the wrapper across the elasticsearch-model and elasticsearch-persistence gems, so the logs/STDERR should be quiet now.


Closed via elastic/elasticsearch-rails#666

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment