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

Openapi docs for File Upload routes #2299

Merged
merged 5 commits into from
Jan 27, 2023
Merged

Openapi docs for File Upload routes #2299

merged 5 commits into from
Jan 27, 2023

Conversation

pzl
Copy link
Member

@pzl pzl commented Jan 25, 2023

What is the problem this PR solves?

This is a followup PR to #1902 to add route documentation to the openapi spec

How to test this PR locally

You may be able to preview the generated output by pasting the contents of model/openapi.yml into the editor side of https://editor.swagger.io/

@pzl pzl added the docs label Jan 25, 2023
@pzl pzl requested review from a team as code owners January 25, 2023 13:43
@elasticmachine
Copy link
Contributor

elasticmachine commented Jan 25, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-01-27T16:23:18.667+0000

  • Duration: 14 min 18 sec

Test stats 🧪

Test Results
Failed 0
Passed 515
Skipped 1
Total 516

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

Copy link
Contributor

@michel-laterman michel-laterman left a comment

Choose a reason for hiding this comment

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

mostly have nitpicks.

model/openapi.yml Outdated Show resolved Hide resolved
model/openapi.yml Show resolved Hide resolved
model/openapi.yml Show resolved Hide resolved
model/openapi.yml Show resolved Hide resolved
responses:
'200':
description: Successful chunk upload
'400':
Copy link
Contributor

Choose a reason for hiding this comment

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

add other responses?

I'm also assuming there's no 428 response anymore

model/openapi.yml Show resolved Hide resolved
model/openapi.yml Show resolved Hide resolved
Copy link
Contributor

@michel-laterman michel-laterman left a comment

Choose a reason for hiding this comment

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

lgtm

- $ref: '#/components/parameters/requestId'
requestBody:
required: true
description: Information about the file to be uploaded. Minimum required fields are marked as required. Additional fields may be specified and are allowed. For information about the file itself, ECS.file paths are recommended. For archived files, information about the archive should be placed in `file`. Information about the archive members may be placed in a `contents` array.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the contents array just an array of objects? Is there any definition for it anywhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah I wasn't sure how much to explain.

The required fields are explicitly mapped. Everything else is unmapped, but dynamic: true is set on the mapping. So integrations can just index whatever they want, in whatever format.

Considering zip/archive is a common use case, I wanted to suggest a convention (have file describe the zip, and contents: [ {}, {}, {} ...] describe the files inside). But it is not required, enforced, anything. Just a suggested convention.

The mappings are not shared between integrations, so any client can do what they want. That's why this isn't a very prescriptive definition

@pzl pzl merged commit 4e9597c into elastic:main Jan 27, 2023
@pzl pzl deleted the file-docs branch January 27, 2023 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants