Add spec for shared folder and link files#888
Conversation
jsoriano
left a comment
There was a problem hiding this comment.
I think that we should take into account the linked files in validations, though they will be taken into account when the package is built, so probably not a blocker till we have more specific validation of source vs. built packages.
Apart from that, I wonder if we should include checksums in the files tested here 🤔
test/packages/with_links/data_stream/foo/fields/some-fields.yml.link
Outdated
Show resolved
Hide resolved
|
Added the logic to be able to do the checks here. I am not sure about what to do with the FindRoot... functionality as it is used in both here and elastic-package, but does not really makes sense to me to have it in here other than that. For now I duplicated it but open to suggestions. I also updated elastic/elastic-package#2483 to make use of the changes |
|
I simplified this as suggested:
For this to be effective I had to set I think once we solve that this seems enough from the spec perspective. |
|
@mrodm I added a semantic check for our case and the pipelines one, not sure this is the way you prefer them to be defined so lmk if any changes are required |
jsoriano
left a comment
There was a problem hiding this comment.
Overall looks good, thanks for adding the suggestions. Added some small questions.
| // It checks the following paths: | ||
| // - agent/input | ||
| // - agent/stream | ||
| // - fields |
There was a problem hiding this comment.
Should we remark that this fields folder is just for the ones defined under data_stream ? Is that right?
If so, would it be better to re-phrase that item in the comment as this? WDYT @marc-gr ?
| // - fields | |
| // - data_stream/*/fields |
IIUC the fields folder in transforms is not taken into account here. If it would be needed those links in transforms, I guess it could be added support in a follow-up. Let's continue with the current links defined in this PR , is that ok @jsoriano?
jsoriano
left a comment
There was a problem hiding this comment.
I opted to add an
allowLinkoption into the item spec after discussing it with @jsoriano as it seems more robust and not rely on the semantic validation, I think this is a cleaner approach together with the custom fs.
Thanks for trying this approach, it looks clearer and I feel more confident merging this. I think there can still be some corner cases, but they will be more limited.
I think this could be merged, but added a couple of questions.
Before merging I would like to double-check what parts of linkedfiles we want/need to publicly expose.
| - description: Add support for _dev/shared folder. | ||
| type: enhancement | ||
| link: https://github.com/elastic/package-spec/pull/888 | ||
| - description: Add support for *.link files in agent, pipelines, and fields folders. | ||
| type: enhancement | ||
| link: https://github.com/elastic/package-spec/pull/888 |
There was a problem hiding this comment.
This should be moved to a new version block for 3.3.6-next.
💚 Build Succeeded
History
|
What does this PR do?
Adds a spec definition for a
sharedfolder under_devthat will allow users to put in there any files that are going to be linked from multiple data_streams and/or packages.Adds a spec definition to allow
*.linkfiles underpipelines,agent, andfieldsto let users reuse repeated files from other parts of the integrations repository.Adds a test package for both.
Checklist
test/packagesthat prove my change is effective.spec/changelog.yml.Related issues