- Notifications
You must be signed in to change notification settings - Fork928
chore: refactor dynamic parameters into dedicated package#18420
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes from1 commit
1ca9483d4dab77c50ee088152bb7247470e679989608bffe2f651edc67644ccea4aa7d758cd26e8f1d2d23840d96328899cd4681321b24d27a1622ddb612b3000f7227b80138c1902a7d2634e221ce54bae82770File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
- Loading branch information
Uh oh!
There was an error while loading.Please reload this page.
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -31,7 +31,7 @@ type Renderer interface { | ||
| Close() | ||
| } | ||
| varErrTemplateVersionNotReady = xerrors.New("template version job not finished") | ||
| // Loader is used to load the necessary coder objects for rendering a template | ||
| // version's parameters. The output is a Renderer, which is the object that uses | ||
| @@ -91,7 +91,7 @@ func (r *Loader) Load(ctx context.Context, db database.Store) error { | ||
| } | ||
| if !r.job.CompletedAt.Valid { | ||
| returnErrTemplateVersionNotReady | ||
| } | ||
| if r.terraformValues == nil { | ||
| @@ -131,7 +131,9 @@ func (r *Loader) Renderer(ctx context.Context, db database.Store, cache *files.C | ||
| // Renderer caches all the necessary files when rendering a template version's | ||
| // parameters. It must be closed after use to release the cached files. | ||
| func (r *Loader) dynamicRenderer(ctx context.Context, db database.Store, cache *files.Cache) (*DynamicRenderer, error) { | ||
| // If they can read the template version, then they can read the file for | ||
| // parameter loading purposes. | ||
| //nolint:gocritic | ||
| fileCtx := dbauthz.AsFileReader(ctx) | ||
| templateFS, err := cache.Acquire(fileCtx, r.job.FileID) | ||
| if err != nil { | ||
| @@ -174,9 +176,8 @@ type DynamicRenderer struct { | ||
| templateFS fs.FS | ||
| plan json.RawMessage | ||
Emyrk marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| ownerErrors map[uuid.UUID]error | ||
| currentOwner *previewtypes.WorkspaceOwner | ||
| once sync.Once | ||
| close func() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -131,4 +131,4 @@ func (r *StaticRender) Render(_ context.Context, _ uuid.UUID, values map[string] | ||
| } | ||
| } | ||
| func (*StaticRender) Close() {} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -68,6 +68,7 @@ func (api *API) templateVersionDynamicParametersWebsocket(rw http.ResponseWriter | ||
| })(rw, r) | ||
| } | ||
| //nolint:revive // listen is a control flag | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Why is a control flag acceptable or desired here if the linter discourages it? Perhaps we should include that justification in the comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Added 👍 Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I understood as much as you've described in the new comment. My question is why this needs to be a "control flag" in this way instead of being extracted from the context inside this endpoint. They're functionally equivalent as far as I can tell. I'm not saying one approach is better. I'm asking because I'm sure the linting rule was added for a reason and MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. As I read it, you are right. I could refactor this to remove the control flag now that the I'm going to keep it for now, and I'm going to make an issue to revisit this. It is on the parameters board, so I can fix this up in my next maintenance cycle 👍 | ||
| func (api *API) templateVersionDynamicParameters(listen bool, initial codersdk.DynamicParametersRequest) func(rw http.ResponseWriter, r *http.Request) { | ||
| return func(rw http.ResponseWriter, r *http.Request) { | ||
| ctx := r.Context() | ||
| @@ -78,16 +79,16 @@ func (api *API) templateVersionDynamicParameters(listen bool, initial codersdk.D | ||
| err := loader.Load(ctx, api.Database) | ||
| if err != nil { | ||
| if httpapi.Is404Error(err) { | ||
| httpapi.ResourceNotFound(rw) | ||
| return | ||
| } | ||
| if xerrors.Is(err, dynamicparameters.ErrTemplateVersionNotReady) { | ||
| httpapi.Write(ctx, rw, http.StatusTooEarly, codersdk.Response{ | ||
| Message: "Template version job has not finished", | ||
| }) | ||
| return | ||
| } | ||
| httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ | ||