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

Daml Script should rely on application id defaulting #13474

Closed
cocreature opened this issue Mar 31, 2022 · 3 comments · Fixed by #13654
Closed

Daml Script should rely on application id defaulting #13474

cocreature opened this issue Mar 31, 2022 · 3 comments · Fixed by #13654
Assignees

Comments

@cocreature
Copy link
Contributor

If users do not set the application id explicitly, we should rely on application id defaulting instead of setting it explicitly to daml-script.

This might need some thinking on how to handle daml script running against 1.x ledgers if we want to support that. One option could be to only do this for user access tokens but not everything.

@sofiafaro-da
Copy link
Contributor

More information on application id defaulting:

Clarification on what the script runner should do:

sofia:
Thanks! Just to clarify, the script runner should leave the application id blank when it isn't passed explicitly? Or it should try to determine what the default application id should be, based on tokens? (edited)

moritz:
I think at least in the case where the user has specified a user token and not specified --application-id it should be left blank. Maybe also in the case where it specifies a custom claims token with an app id.
For unauthorized usage I think you do still need it.

@akrmn
Copy link
Contributor

akrmn commented Apr 20, 2022

@cocreature

moritz: [...] --application-id should be left blank

Do you mean blank as in the empty string, or should the type of LedgerClientConfiguration.applicationId change to Option[String]?

@cocreature
Copy link
Contributor Author

Do you mean blank as in the empty string, or should the type of LedgerClientConfiguration.applicationId change to Option[String]?

it’s protobuf so empty string is identical to not setting it. But maybe a good idea to change that API.

@akrmn akrmn linked a pull request Apr 21, 2022 that will close this issue
@akrmn akrmn self-assigned this Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants