Skip to content

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

Merged
merged 9 commits into from
Dec 9, 2022
Merged

feat: add examples to api #5331

merged 9 commits into from
Dec 9, 2022

Conversation

f0ssel
Copy link
Contributor

@f0ssel f0ssel commented Dec 6, 2022

What this does:

  • Moves examples.Example to codersdk.TemplateExample
  • Adds route at GET /organizations/{org}/templates/examples that returns[]codersdk.TemplateExample
  • Adds optional field ExampleID on codersdk.CreateTemplateVersionRequest
  • Support ExampleID in api.postTemplateVersionByOrganization in favor of FileID

@f0ssel f0ssel requested a review from a team as a code owner December 6, 2022 21:45
@f0ssel f0ssel requested review from presleyp and BrunoQuaresma and removed request for a team and presleyp December 6, 2022 21:45
@f0ssel f0ssel requested a review from deansheather December 7, 2022 16:43
Copy link
Member

@deansheather deansheather left a 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

@f0ssel f0ssel requested review from deansheather and a team December 7, 2022 20:11

// 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)
Copy link
Member

@deansheather deansheather Dec 8, 2022

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.

Copy link
Contributor Author

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?

@BrunoQuaresma
Copy link
Collaborator

@f0ssel can we add a new field called Tags or Categories for filtering?

@BrunoQuaresma
Copy link
Collaborator

Also, we need an endpoint to fetch one single example. Something like /examples/{exampleId}

@BrunoQuaresma
Copy link
Collaborator

Another thing, sorry 😅, I'm missing the icon field.

@f0ssel
Copy link
Contributor Author

f0ssel commented Dec 8, 2022

@f0ssel can we add a new field called Tags or Categories for filtering?

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.

Also, we need an endpoint to fetch one single example. Something like /examples/{exampleId}

What additional data would you expect to return here, that /template/examples isn't already returning?

Another thing, sorry 😅, I'm missing the icon field.

Okay, I can see if I can grab this by adding something to the examples package on the backend.

@BrunoQuaresma
Copy link
Collaborator

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.

Good question. I really don't know but getting it from the markdown, as you said, using frontmatter sounds reasonable.

What additional data would you expect to return here, that /template/examples isn't already returning?

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.

Okay, I can see if I can grab this by adding something to the examples package on the backend.

Nice! Maybe this could be a markdown thing as well?

@f0ssel
Copy link
Contributor Author

f0ssel commented Dec 8, 2022

What additional data would you expect to return here, that /template/examples isn't already returning?

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.

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.

@f0ssel
Copy link
Contributor Author

f0ssel commented Dec 8, 2022

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.

Good question. I really don't know but getting it from the markdown, as you said, using frontmatter sounds reasonable.

I looked and see the tags yaml in the markdown files. I'll parse these and expose them through the API as well.

@f0ssel
Copy link
Contributor Author

f0ssel commented Dec 9, 2022

@BrunoQuaresma both tags and icon are in the API now. Currently none of the templates have icon urls but they can easily be added to the markdown if you have them.

Co-authored-by: Dean Sheather <dean@deansheather.com>
@f0ssel f0ssel merged commit ca0374b into main Dec 9, 2022
@f0ssel f0ssel deleted the f0ssel/examples branch December 9, 2022 19:29
@github-actions github-actions bot locked and limited conversation to collaborators Dec 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants