dashboard: session logs, events logs, integration module #315
Conversation
There was a problem hiding this comment.
only super sophisticated users will be able to use the sdk
it would be much easier to use the protos natively instead of using all of this wrappers
I really think we should rethink the SDK
provide a super super ultra simple SDK to the client and stop using the SDK in the backend and use simpler less abstracted / wrapped code
There was a problem hiding this comment.
I didn't really review the dashboard code and web content, because I consider it to be throwaway code.
only super sophisticated users will be able to use the sdk
it would be much easier to use the protos natively instead of using all of this wrappersI really think we should rethink the SDK
provide a super super ultra simple SDK to the client and stop using the SDK in the backend and use simpler less abstracted / wrapped code
I agree with Efi for slightly different reasons:
The rule of thumb in Google was to never create libraries that wrap proto services - because then you have to maintain them for multiple languages, whereas generated proto clients are universal.
Also, the result is that our SDK contains, or more precisely hides, all the validation logic from the server side. It would make more sense to me to put all validation in the proto server instead of promising to it that the input is valid. That promise is both dangerous in the long term when we don't have a single person who owns this code, and has already created a steep learning curve for our codebase.
I also suspect that our heavy reliance on generics in the SDK is... less than efficient. But we don't have the time to check this.
|
|
||
| document.querySelectorAll('.jsoneditor').forEach((element) => { | ||
| let content = { json: JSON.parse(element.innerText), text: undefined } | ||
| element.innerHTML = '' |
There was a problem hiding this comment.
Reminder, not affecting this PR: https://linear.app/autokitteh/issue/ENG-788/solve-xss-issue-with-dashboard-messages
|
i guess i need to do another once over the SDK, but not in the very near future |
No description provided.