-
Notifications
You must be signed in to change notification settings - Fork 888
feat: add examples to api #5331
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs tests but code LGTM
|
||
// TemplateExamples lists example templates embedded in coder. | ||
func (c *Client) TemplateExamples(ctx context.Context, organizationID uuid.UUID) ([]TemplateExample, error) { | ||
res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/organizations/%s/templates/examples", organizationID), nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are template examples org-scoped? Seems like you did it so it could be /templates/examples
but I maybe you should use /api/v2/template-examples
instead or something else since it's not org scoped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are admin only so I felt it was better to keep it all under that same auth schema. Do you think it really impacts anything until we have multi-org?
@f0ssel can we add a new field called |
Also, we need an endpoint to fetch one single example. Something like |
Another thing, sorry 😅, I'm missing the icon field. |
Where would we populate this data from? I don't think we can do much manually, but we could possibly parse the markdown or something.
What additional data would you expect to return here, that
Okay, I can see if I can grab this by adding something to the |
Good question. I really don't know but getting it from the markdown, as you said, using frontmatter sounds reasonable.
Yes, all the data is there but let's say the user navigates to the "aws-docker" example page and it has to fetch all the examples, which I'm considering can grow up fast, just to get one result. I think it could be better to have an endpoint to just return an example by ID so it works well independent of the number of examples we have. Not a big thing tho, whatever you think is better.
Nice! Maybe this could be a markdown thing as well? |
This dataset is not crazy large is size and static once loaded into memory on app startup. Returning the entire set is already how the package api is made, and it's essentially free for us to return, so I think it would be better to just load the entire set into context once and use it to show separate info pages as well. |
I looked and see the |
@BrunoQuaresma both |
Co-authored-by: Dean Sheather <dean@deansheather.com>
What this does:
examples.Example
tocodersdk.TemplateExample
GET /organizations/{org}/templates/examples
that returns[]codersdk.TemplateExample
ExampleID
oncodersdk.CreateTemplateVersionRequest
ExampleID
inapi.postTemplateVersionByOrganization
in favor ofFileID