resin-sdk constructor overrides settings

[zvin]

Here https://github.com/resin-io/resin-sdk/blob/master/lib/resin.coffee#L77 The sdk sets a default value for apiUrl and imageMakerUrl instead of getting these from settings.

This means that if you run a script that uses resin-sdk with the env var RESINRC_RESIN_URL set to resinstaging.io,

• resin.models.application.get(...) will use resin.io (the resin-sdk default)
• resin.settings.getAll() will return settings using resinstaging.io

We probably should remove the sdk apiUrl and imageMakerUrl parameters and use the values provided by resin-settings-client.

resin-cli doesn't have this problem because it uses resin-sdk-preconfigured which works around the issue: https://github.com/resin-io-modules/resin-sdk-preconfigured/blob/master/src/index.ts

[pimterry]

Totally agree with the problem, but I think the solution is the opposite, instead of

We probably should remove the sdk apiUrl and imageMakerUrl parameters and use the values provided by resin-settings-client.

we should isolated the SDK, and ignore the shared settings completely, taking its configuration just from the given arguments.

If you want to use system-wide settings from resin-settings-client you can easily opt into that, and if you don't you can easily set them explicitly in an idiomatic JS way (options arg). Gives you more flexibility that way, and makes fewer assumptions about the environment (which is particularly helpful in browser land, where having to mock process.env.RESINRC_RESIN_URL or instantiate resin-settings-client by hand to configure things before using the SDK would be super painful).

[zvin]

@pimterry this won't solve my problem if we can access resin-settings-client through resin.settings. If RESINRC_RESIN_URL is defined, resin.settings will use it and you'll get different hosts in resin.settings and in what resin.models uses.

[pimterry]

Oooh, interesting, you're totally right. resin.settings is inherently exposing global state, and the rest of the SDK doesn't want to.

Should resin.settings really live in the SDK? There's two conflicting models here (configured & isolated vs controlled by/controlling global state) and simply removing that (so you'd use resin-settings-client etc directly) would solve that... Awkward, but it does seem weird that our tool for talking to the API also thinks about how we persist configuration on your machine.

Alternatively, could we instead make the system-wide settings the defaults, and only override if you explicitly pass in an argument? If we do that then everything will agree by default (if you pass nothing in), and if you pass something in then resin.settings will still give you the system settings that differ from the used ones, but maybe that's fine...

No, that's probably not fine, but arguably that's a matter of presentation - if this was resin.globalSettings/storedSettings/systemSettings then it might be clearer what was going on?

[zvin]

Then maybe the simplest solution is to remove resin.settings completely? This will avoid any possible confusion and you can still require resin-settings-client if you need it.