Skip to content
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

Figure out validation / default value implementation for JS #29

Closed
georgesnelling opened this issue Nov 19, 2019 · 1 comment
Closed
Labels
enhancement New feature or request spec-first This issue needs a spec to be created and then work. It should be an independent stream ui Admin console user interface

Comments

@georgesnelling
Copy link
Contributor

georgesnelling commented Nov 19, 2019

Problem:

The messages coming back from the API are decoded by protobufjs. But since all the fields in a proto messages are optional by convention, we don't have any assurance that the records are valid and usable. This has caused errors before on the client side.

 

Solution options:

  • Manual validation of the records and type-casting (message as X) or type-guarding (: message is X) to the stricter types present on the client side. This has the advantage of being flexible in the UI requirements, and the disadvantage of being difficult to keep in sync with the protobuf source of truth.
  • Automated validation via some type of schema definition stored on the client side (JSON Schema is one such option). This has the advantage of generating consistent code on the client side which is kept up-to-date automatically as the schema is updated, as well as providing a schema document that can be used to validate the JSON output from the API. It has the same disadvantage of being a separate solution which must be updated manually any time the API contract changes.
  • Switch the console to use protoc-generated JS/TS libraries and decorate all protobuf messages with the appropriate validation. This has the advantage of the validation rules being identical on both server and client (and updating automatically) as well as providing a generic solution for validation (call validate() on the message class coming back from the server). It has the disadvantage of requiring a non-trivial amount of work: Switching from protobufjs to protoc, enabling the TS output from protoc, updating console code to work with the new typings and decoding strategy.

Option 3 is ideal, but the amount of work necessary to do so is concerning (especially considering it may not work correctly and we might have to back it out).

@schottra schottra added the ui Admin console user interface label Dec 11, 2019
@schottra schottra added enhancement New feature or request spec-first This issue needs a spec to be created and then work. It should be an independent stream and removed Compliance pri2 labels Jul 6, 2020
@schottra schottra removed their assignment Jul 6, 2020
@EngHabu EngHabu closed this as completed Jan 29, 2021
@EngHabu
Copy link
Contributor

EngHabu commented Jan 29, 2021

Bug bankruptcy.

eapolinario referenced this issue in eapolinario/flyte Dec 20, 2022
* Work in progress: Generic Stowstore

* Simplifying to use only stowstore

* added missing functions

* removing deleted reference

* Updated pflags

* Fixed
eapolinario referenced this issue in eapolinario/flyte Dec 20, 2022
eapolinario referenced this issue in eapolinario/flyte Aug 21, 2023
* Work in progress: Generic Stowstore

* Simplifying to use only stowstore

* added missing functions

* removing deleted reference

* Updated pflags

* Fixed
eapolinario referenced this issue in eapolinario/flyte Aug 21, 2023
eapolinario referenced this issue in eapolinario/flyte Aug 21, 2023
Signed-off-by: Prafulla Mahindrakar <[email protected]>
eapolinario pushed a commit that referenced this issue Sep 12, 2023
austin362667 pushed a commit to austin362667/flyte that referenced this issue May 7, 2024
robert-ulbrich-mercedes-benz pushed a commit to robert-ulbrich-mercedes-benz/flyte that referenced this issue Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request spec-first This issue needs a spec to be created and then work. It should be an independent stream ui Admin console user interface
Projects
None yet
Development

No branches or pull requests

3 participants