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

API V2: Consider using "URI" in the ModelServerClient API instead of "string" #191

Open
CamilleLetavernier opened this issue Mar 17, 2022 · 1 comment
Labels
enhancement New feature or request modelserver EMF.cloud Model Server v2

Comments

@CamilleLetavernier
Copy link
Member

We often have inconsistent encoding in URIs, which isn't usually an issue on Linux (Since we don't typically use special characters), but fail on Windows (Because : is a special character, and is used in all absolute file paths: c:/...).

URIs shouldn't be pre-encoded when passed the the ModelServerClient API, otherwise they will be double-encoded. However, the default behavior of URI.toString() is to encode the URI, which makes it an easy mistake for all users of the ModelServerClient API.

There are two things we can do to limit this risk:

  • Clarify in the documentation that the modeluri is an un-encoded URI string.
  • Add an option to use the URI object directly in the API, instead of an arbitrary string. This should be the preferred option in most cases, as the ModelServerClient implementation then becomes responsible for encoding (or not) the URI as needed

At the moment, we need to make sure to use uri.toString(skipEncoding=true) with all ModelServerClient APIs to avoid URI encoding issues. For example:

const uri: URI = getURI();
const modeluri: string = uri.toString(/* skipEncoding =*/ true);
const patch = getPatch();
modelServerClient.edit(modeluri, patch);
@CamilleLetavernier CamilleLetavernier added enhancement New feature or request modelserver EMF.cloud Model Server labels Mar 17, 2022
@eneufeld
Copy link
Contributor

+1 for passing URI instead of string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request modelserver EMF.cloud Model Server v2
Projects
None yet
Development

No branches or pull requests

3 participants