Making the SDK suitable for the browser about balena-sdk HOT 7 CLOSED

Hi Eugene,

The settings issue is one that has been there for quite a lot of time,
mainly due to historical reasons, since the SDK was "built" in the CLI.

All the points you make absolutely make a lot of sense. Ideally, the
whole settings stuff should live in the CLI, not in the SDK, and
settings would be simply passed as an object to the SDK.

In fact, I've started the work to completely decouple the SDK from the
settings in this PR long ago:
#193 but never actually
finished due to the decoupling not being an immediate goal at that
moment. My suggestion is to check that PR out, and see if you can
resolve the conflicts (its from February) and actually get it merged,
which should do most of the work for you.

The PR introduces various changes that align with what you've suggested:

• The SDK stops reading settings from the file-system (from
  resin-settings-client, etc), and instead gets passed a plain object.
• The baseUrl is explicitly passed to every resin-request call, so
  that module stops depending on the settings mess.
• Decouple resin-pine from the settings as well.

And some other stuff that I don't really remember anymore. You can check
the commit messages on that PR for more details.

Does that help?

On Sat, Sep 10, 2016 at 01:18:40PM -0700, Eugene Mirotin wrote:

As you know I've recently started investigating some lower-level modules and improving them to be browser-compatible and adding tests.

Now when I've reached the resin-pine module I've realized the problem is bigger than I assumed.
Both the CLI and the SDK are highly modular, but there's a global design issue that prevents the SDK from working in the browser as is.
The issue is that multiple low-level modules are actually not universal libraries because they keep knowledge about how things (mostly settings) are stored on the file system.

I've built a diagram to illustrate the modules relations: https://ns-pxnflxuoaj.now.sh/resin-sdk%20modules.html. Note that I've excluded many modules that are universal and are not part of the problem.

So in the ideal world, the SDK is supposed to be a universal library. Whatever is specific to the way the CLI works (like storing the apiUrl in the settings file) should only be known to the CLI itself. The SDK should accept all the information it needs as options.

Let's look at an example.
https://github.com/resin-io-modules/resin-pine is a nice library that provides all the sane params to the pinejs-client. But it has several flaws that prevent it from being browser-compatible. The main problem as mentioned above is that here https://github.com/resin-io-modules/resin-pine/blob/master/lib/pine.coffee#L57 it reads the apiUrl from the file system. In the browser there's even no analog of that. The URL is not stored in the localStorage. Instead, it's a constant in the code that should be used to instantiate the SDK. The CLI would still use the same settings library to read the URL (on its own) and then again pass it to the SDK.

There're a couple more issues of the similar nature:

• here https://github.com/resin-io-modules/resin-pine/blob/master/lib/pine.coffee#L57 the library talks to a different library to check some data, but doesn't actually use it (it throws if the token is missing, but the actual token is not used). It's a small thing, but it's another example of the knowledge shared across the libraries. In my understanding, the library should just pass the options to the resin-request and let it complain (BTW some endpoints may not require auth).
• here https://github.com/resin-io-modules/resin-pine/blob/master/lib/pine.coffee#L52 the library relies on the process.env which is Node-specific.
• in the same line the RESIN_API_KEY variable is checked, but it's never used. It's confusing to find where it is actually read and passed to the resin-request which expects it as part of the options.

On the contrary, if we check the https://github.com/resin-io-modules/resin-register-device library we'll see a great example of universal code. It requires the user to pass it a PineJS instance and thus is completely agnostic of the way that instance is created and configured.

---

Given these examples here's how I see the plan to make the SDK universal:

• resin-token will remain as it is (now that it can run in both node and the browser), and will be the only low-level library directly dealing with storage (because we want it to persist the token without our intervention).
• as far as I can see resin-settings-client is currently serving two purposes: it keeps some useful defaults and shared info, and it also reads the user overrides from the file system. It has to be split somehow so that the first part can be used in the browser (if ever applicable).
• resin-pine, resin-request will be made independent from resin-settings-client and process.env. Whatever they need should be passed to them as options. These are the modules I've found, but there can be more. This is a breaking change and will require the major versions to be released.
• resin-sdk is made agnostic as well. It should receive the entire set of options, and pass them to sub-modules accordingly. Again it will require a new major version.
• Finally, resin-cli should be responsible for reading he options (using resin-settings-client) and passing them to the SDK.

Then we'll be able to make the UI do the same but use its own means to get the properties (for example, the api key there will always be undefined, and the api URL is part of the UI code, etc.).

You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub:
#221

Juan Cruz Viotti
Software Engineer

from balena-sdk.

Thanks. I'll check that PR and will either rebase it or recreate it in a similar way.

from balena-sdk.

I think I've finally made it work. Going to send PRs to individual repos in the next couple of days.

from balena-sdk.

Updated dependency diagram: https://resin-sdk-modules-kpjhjrzoxv.now.sh/

Collecting the published versions of the components here (has to be checked):

• [email protected]
• [email protected]
• [email protected]
• [email protected]
• [email protected]
• [email protected]
• [email protected]
• [email protected]

from balena-sdk.

@emirotin good to close?

from balena-sdk.

from balena-sdk.

Cool, closing :)

from balena-sdk.