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.

Sign up for GitHub

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

[dev guide] Add docs on optional schema arguments #12371

Merged
merged 6 commits into from
Jun 5, 2019
Merged

[dev guide] Add docs on optional schema arguments #12371

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
65d2cc1
add docs on optional schema arguments
fearful-symmetry May 30, 2019
ca869dd
remove extra newlines
fearful-symmetry May 30, 2019
cb799a6
clean up grammar
fearful-symmetry May 31, 2019
aabbe56
put IgnoreAllErrors in the right place, add ApplyTo docs
fearful-symmetry May 31, 2019
01eccba
example syntax
fearful-symmetry May 31, 2019
ba5720c
fix typos, error handling in example
fearful-symmetry Jun 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions docs/devguide/create-metricset.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ common.MapStr{
}
----

You can use the following code to make the transformations:
You can use the schema package to transform the data, and optionally mark some fields in a schema as required or not. For example:

[source,go]
----
Expand All @@ -231,9 +231,9 @@ import (

var (
schema = s.Schema{
"test_string": c.Str("testString"),
"test_int": c.Int("testInt"),
"test_bool": c.Bool("testBool"),
"test_string": c.Str("testString", s.Required), <1>
"test_int": c.Int("testInt"), <2>
"test_bool": c.Bool("testBool", s.Optional), <3>
"test_float": c.Float("testFloat"),
"test_obj": s.Object{
"test_obj_string": c.Str("testObjString"),
Expand All @@ -242,12 +242,19 @@ var (
)

func eventMapping(input map[string]interface{}) common.MapStr {
return schema.Apply(input)
return schema.Apply(input) <4>
}
----
<1> `Required` will explicitly mark a field as non-optional.
fearful-symmetry marked this conversation as resolved.
Show resolved Hide resolved
<2> If a field has no Schema Option set, it is equivalent to `Required`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why "Schema Option" is capitalized here. Generally we only capitalize proper nouns. I would change this to say, "If no schema option is set, the field is required."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More than likely, I was looking at the code, which is capitalized, and my brain just translated it to words that way, hah.

<3> `Optional` You can also explicitly mark a field as not required.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<3> `Optional` You can also explicitly mark a field as not required.
<3> Marks the field as optional.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand why you are being precise here, but I think the content is harder to read when you swap between using "non-optional" and "not required." Except in situations where it's important to know that a setting is explicit (like you do later on where you say "field explicitly marked as required"), I would simply refer to a fields as required or optional. I'll suggest edits with this notion in mind. I hope I'm understanding things correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yah, I was struggling to word that in a way that didn't seem repetitive.

<4> By default, `Apply` will fail and return an error if any field not marked as optional is missing. Using the optional second argument, you can specify how `Apply` handles different fields of the schema. The possible values are:
fearful-symmetry marked this conversation as resolved.
Show resolved Hide resolved
- `AllRequired` is the default behavior. Return an error if any field not marked as optional is missing.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `AllRequired` is the default behavior. Return an error if any field not marked as optional is missing.
- `AllRequired` is the default behavior. Returns an error if any required field is missing.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to disambiguate further, you could add "...any required field is missing, including fields that are required because no schema option is set."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yah, that's what I was trying to get across. Thanks!

- `FailOnRequired` will fail if a field explicitly marked as `required` is missing.
- `NotFoundKeys(cb func([]string))` will pass a callback function that will be called with the list of missing keys, for more fine-grained error handling.
Copy link
Contributor

Choose a reason for hiding this comment

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

This sentence was a little hard to parse on first reading. Would it be accurate to say "...will pass a callback function that returns a list of missing keys for more fine-grained error handling."


In the above example, note that it is possible to create the schema object once
and apply it to all events.
and apply it to all events.

[float]
==== Configuration File
Expand Down