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

Marks items as arrays if required. #661

Closed
wants to merge 1 commit into from
Closed

Conversation

codebrain
Copy link
Contributor

This is to aid code generation for languages that support array constructs.

Note: This field is optional, the default value is false. Lack of field presence implies a value of false.

This is to aid code generation for languages that support array constructs.

Note: This field is optional, the default value is false. Lack of field presence implies a value of false.
@codebrain codebrain requested a review from webmat December 2, 2019 03:35
@webmat
Copy link
Contributor

webmat commented Dec 3, 2019

Thanks for getting this started, @codebrain. This will be useful. :-)

@webmat
Copy link
Contributor

webmat commented Dec 3, 2019

@codebrain If you don't mind I'll take over the PR from here. That was the plan, right? 🙂

I'll change the way we're specifying what's an array to something like:

normalize:
  - array

For now I will not add more under normalize, but eventually I'd like to formalize other kinds of normalization (e.g. lower/upper casing, perhaps date formats) with this...

The purpose of this PR is still 100% focused on clarifying what's an array, however. So:

  • marking all the fields that should be arrays
  • tweaking the code to display the info in the docs
  • having the information show up in the generated/ecs yaml files

@webmat webmat self-assigned this Dec 3, 2019
@webmat webmat removed their request for review December 3, 2019 20:47
@webmat
Copy link
Contributor

webmat commented Dec 3, 2019

Not quite ready to update this PR yet, but I've started work on adapting it here https://github.com/webmat/ecs/commits/is-array-markers, if you want to check it out, @codebrain

@codebrain
Copy link
Contributor Author

webmat@843e521 - looks good.

I didn't realise, but it looks as if there are more arrays. I will wait for this to be merged in and then tweak our generator for 1.3.

Thanks Mat, yes, more than happy for you to pick this up from here on in :)

@jeskew
Copy link

jeskew commented Dec 30, 2019

@webmat do you know if there's a target release for webmat@843e521 to be incorporated into master? I use ECS for a system that tees logs into Elasticsearch and parquet files and have been maintaining a separate manifest of which fields should be treated as arrays in the parquet schema.

@webmat
Copy link
Contributor

webmat commented Jan 3, 2020

@jeskew We can't make any promises on the release of this branch. But it's towards the top of the list of priorities, as many folks need this, like you do. I'm hoping it will make it into the next ECS release.

Note also that merging something to master doesn't make it official. The master branch is a development branch. Only the releases make new developments official 😉

jeskew-gov added a commit to jeskew/elastic-common-schema-smithy that referenced this pull request Jan 8, 2020
Lucene treats all fields as lists, so Elasticsearch/ECS's schema IDL
does not distinguish between fields that should be lists and those
that should not. This distinction matters for everything not based
on Lucene indices.

Until elastic/ecs#661 lands in an official
ECS release, the model builder will identify which fields should be
rendered as Smithy lists based on an internal manifest.
@webmat webmat closed this in #727 Feb 6, 2020
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