search

digital-asset / daml

The Daml smart contract language
https://www.digitalasset.com/developers
Other
802 stars 204 forks source link

warn when user management is requested by client while unsupported by server #12173

Open adriaanm-da opened 2 years ago

adriaanm-da commented 2 years ago

should have a warning if an unexpected ledger api server version/feature descriptor is encountered

cocreature commented 2 years ago

We’re not changing the ledger API version to 2.0 afaik? I thought it’s only a minor bump

adriaanm-da commented 2 years ago

good point, I was converting draft issues to github so we can track dependencies -- this needs to be fleshed out and refined

adriaanm-da commented 2 years ago

Depends on #12174

cocreature commented 2 years ago

What exactly is the request here? Is it not acceptable if daml script or the JSON API fail with "Method not implement" if you call a user mgmt function on a ledger that does not support user mgmt in 2.0?

adriaanm-da commented 2 years ago

That would be acceptable, but it would be better if we had a clearer warning. It's a better user experience, and may simplify support if we can more easily tell the root cause (version mismatch between server and client).

cocreature commented 2 years ago

In that case, do you want to check for the version or for the feature descriptor? The latter seems more precise and I think I don’t fully understand how the version will be on vmbc without user management for example.

adriaanm-da commented 2 years ago

Yeah, good point -- I think this particular example (missing functionality: user mgmt) would be better served by the feature descriptor. More broadly speaking, and paraphrasing @meiersi-da, who suggested this, having some version checks would also be useful for diagnostics/preventing confusion.

cocreature commented 2 years ago

Makes sense, I’d suggest splitting it up in 2 issues

  1. Handling for user management in particular. Parts of which we might want to have in 2.0 although I’d argue "method not implemented" is good enough to not be a release blocker.
  2. General warnings (but not errors except for maybe specific endpoints) for ledger API version mismatches between client & server. I don’ tsee any reason why this has to be in 2.0 given that this isn’t a new problem by any means.
adriaanm-da commented 2 years ago

I tried to make the title more specific, and split off the version diagnostic as a separate ticket. As I agree this is a refinement and not a release blocker, this can be scheduled post 2.0.