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

List available schemas in 404 response to /schemas #3469

Closed

Conversation

chalin
Copy link
Contributor

@chalin chalin commented Nov 1, 2023


Preview: https://deploy-preview-3469--opentelemetry.netlify.app/schemas/

Screenshot (mobile)

image

@chalin chalin requested a review from a team November 1, 2023 19:41
@chalin chalin force-pushed the chalin-im-schemas-index-2023-11-01 branch from 428dae0 to 92c8871 Compare November 1, 2023 19:43
@chalin
Copy link
Contributor Author

chalin commented Nov 1, 2023

@tigrannajaryan
Copy link
Member

I think it is a good idea to point /schemas/latest to the latest published schema.

I don't think we should publish a human-readable list at /schemas/. My previous argument still holds: if we want /schemas/ to contain anything it should be a machine-readable format defined in Otel spec. Since we don't have such format defined I think we should not publish anything at the index yet.

@chalin chalin mentioned this pull request Nov 1, 2023
2 tasks
@chalin
Copy link
Contributor Author

chalin commented Nov 1, 2023

Thanks for your response @tigrannajaryan - see #3468 (comment) for a clarification of the current situation (/schemas/ already returns human readable HTML) and two suggested ways for us to have our cake and eat it too 🎂 😄.

@chalin chalin force-pushed the chalin-im-schemas-index-2023-11-01 branch from 5051312 to 02e8a37 Compare November 1, 2023 22:38
@chalin chalin marked this pull request as draft November 1, 2023 23:43
@chalin chalin force-pushed the chalin-im-schemas-index-2023-11-01 branch from 3a7c207 to 07acb2d Compare November 1, 2023 23:59
@chalin
Copy link
Contributor Author

chalin commented Nov 1, 2023

Converting to draft until we reach a consensus at #3468.

In the meantime, I'm testing the option of returning a 404 status for the schema index page, while still providing the list of schemas in human readable form.

@chalin chalin mentioned this pull request Nov 2, 2023
@chalin
Copy link
Contributor Author

chalin commented Nov 2, 2023

Ok, I'm going to split this up so as to not hold back the individual fixes. Here's the first:

@chalin
Copy link
Contributor Author

chalin commented Nov 2, 2023

Here's the other feature factored out:

I'll repurpose this PR to be only about the index page.

@chalin chalin changed the title Schemas: add latest redirect, index page, fix content-type Schemas: add index page Nov 2, 2023
@chalin chalin force-pushed the chalin-im-schemas-index-2023-11-01 branch from 07acb2d to 5e9f962 Compare November 2, 2023 12:21
@chalin chalin force-pushed the chalin-im-schemas-index-2023-11-01 branch from 5e9f962 to 02d1844 Compare November 2, 2023 14:48
@chalin chalin changed the title Schemas: add index page List available schemas in 404 response to /schemas Nov 6, 2023
@chalin
Copy link
Contributor Author

chalin commented Nov 14, 2023

@chalin chalin closed this Nov 14, 2023
@chalin chalin deleted the chalin-im-schemas-index-2023-11-01 branch November 14, 2023 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants