Add support for serving the static assets in a report #140
Conversation
| NoTxn(context.Context) db.Tx | ||
|
|
||
| AnalysisArtifactsForAnalysis(tx db.Tx, id pacta.AnalysisID) ([]*pacta.AnalysisArtifact, error) | ||
| Blobs(tx db.Tx, ids []pacta.BlobID) (map[pacta.BlobID]*pacta.Blob, error) |
There was a problem hiding this comment.
I think you're planning this for the next PR, if so great, but we'll need to generate Audit Logs here.
| s.logger.Error("path had no UUID prefix", zap.String("analysis_artifact_id", string(a.ID)), zap.String("blob_uri", uri), zap.String("blob_path", path)) | ||
| continue | ||
| } | ||
|
|
There was a problem hiding this comment.
One piece I don't love about this approach is that if N = number of assets in a given report, we read back N rows 2*N times to serve the report. I get that this is still linear in the # of DB calls made, it just seems really duplicative, since the returns for each of these DB calls is identical. I can't think of a simple way of sidestepping this property, but I don't love it.
There was a problem hiding this comment.
Yeah, I also didn't like that and considered doing a dumb in-mem cache of analysis ID -> artifacts, but the cardinality is low (e.g. 10-20 artifacts), and the loading speed isn't a problem. Can revisit if it becomes an issue.
| return | ||
| } | ||
|
|
||
| func fileTypeToMIME(ft pacta.FileType) string { |
There was a problem hiding this comment.
I guess this is one good reason to have FT saved!
There was a problem hiding this comment.
Indeed! As discussed, I'll update the parent PR to store unknown as 'unknown'
bcspragu
force-pushed
the
brandon/serve-report
branch
from
e2d50a5 to
41fedbc
Compare
This PR adds a new `/report/<analysis ID>` endpoint that serves a report. Importantly, this is a _server_ endpoint, our Nuxt client knows nothing about it. That means to test it locally, you go to `localhost:8081/report/<analysis ID>`, and when deployed, it'll be `https://pacta-api.dev.rmi.siliconally.dev/report/<analysis ID>` IIRC. Since we share cookies between `*.dev.rmi.siliconally.dev`, this works fine. The main thing missing from this PR is authz, which will be the next thing I do.
bcspragu
force-pushed
the
brandon/serve-report
branch
from
41fedbc to
23940d7
Compare
This PR adds a new
/report/<analysis ID>endpoint that serves a report. Importantly, this is a server endpoint, our Nuxt client knows nothing about it.That means to test it locally, you go to
localhost:8081/report/<analysis ID>, and when deployed, it'll behttps://pacta-api.dev.rmi.siliconally.dev/report/<analysis ID>IIRC. Since we share cookies between*.dev.rmi.siliconally.dev, this works fine.The main thing missing from this PR is authz, which will be the next thing I do.