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

ES Reader: Handle missing fields when reading from ES #261

Conversation

justin-stephenson
Copy link
Collaborator

When playing back from elasticsearch, add empty placeholder value for
fields(in_txt, host, user) if these fields are missing in ES database.

@justin-stephenson justin-stephenson force-pushed the es_play_handle_missing_fields branch 2 times, most recently from 5182bc2 to c9a1f82 Compare August 6, 2019 17:43
@coveralls
Copy link

coveralls commented Aug 6, 2019

Coverage Status

Coverage increased (+0.004%) to 29.412% when pulling 11419dc on justin-stephenson:es_play_handle_missing_fields into d1cad1f on Scribery:master.

Copy link
Member

@spbnick spbnick left a comment

Choose a reason for hiding this comment

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

This is a good start, but it has a few problems. First of all users are likely to be surprised and confused if only the ES backend handled the missing fields. Instead we need to change the schema, and documentation to describe the optional fields, what they would default to, and change the json_msg.c to handle it.

Another side is the choice of the optional fields. I think the "host" and "user" can't normally be empty strings and so they'll not be omitted in ES. Instead we need to handle "in_txt" and "out_txt" being absent.

Allow playing back recordings when in_txt/out_txt fields are missing.
An empty string "" value will be used as a default if these fields are
absent. This handles missing fields when reading from ElasticSearch
or other backends.
@justin-stephenson
Copy link
Collaborator Author

Thank you Nikolai, I made those changes. The schema.json already excludes in_txt and out_txt from the required properties list.

@justin-stephenson justin-stephenson merged commit b3f0801 into Scribery:master Aug 26, 2019
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.

None yet

3 participants