Integrate reporter #1558
Integrate reporter #1558
Conversation
peraltafederico
force-pushed
the
feat/reporter
branch
from
772ef2f
to
9afa8ab
Compare
peraltafederico
requested review from
pastuxso and
sourishkrout
and removed request for
pastuxso
| metadata: { ['runme.dev/frontmatter']: string }, | ||
| kernel?: Kernel, | ||
| ): Frontmatter { | ||
| const yamlDocs = YAML.parseAllDocuments(metadata['runme.dev/frontmatter']) |
There was a problem hiding this comment.
Hey @peraltafederico, should we consider offering support for frontmatter that isn't in YAML format?"
There was a problem hiding this comment.
Good question, @pastuxso! Initially, I started parsing the metadata with gray-matter, but since we were already using yaml, I thought it would be a good idea to stay consistent.
I think what matters most now is how our current design aligns with this approach, so using yaml should be fine for now, but I’ll leave the final decision to @sourishkrout.
There was a problem hiding this comment.
@peraltafederico do we have to serialize the frontmatter? runme.parser.v1.Frontmatter is well defined. If serialization is required on the backend side maybe that's where it should happen?
There was a problem hiding this comment.
@sourishkrout Just to confirm, you mean that I just parse the raw frontmatter in the Platform APi instead of doing it when marshaling the Notebook, right?
There was a problem hiding this comment.
@sourishkrout I just realized I'm marshaling the frontmatter to also get the session ID, see:
vscode-runme/src/extension/serializer.ts
Line 829 in b56b841
I'm leaving this as is for now, but please let me know if there's a different approach I may be overlooking!
There was a problem hiding this comment.
Okay, @peraltafederico. We can leave it in place for now. It's in the protos but if we skip out on the Kernel reporter API for now, that's no help.
|
Normalize the Platform payload based on the new Reporter message definition from the Go kernel. Currently in this PR, the extension manually parses a notebook to create a valid Platform payload, but in the near future it will delegate that parsing to the Reporter service.
Depends on stateful/runme#655 and https://github.com/stateful/platform/pull/822.
Tests are failing until the proto files are generated.