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

Figure out how to initialize viaduct for nimbus #3592

Closed
data-sync-user opened this issue Sep 30, 2020 · 7 comments
Closed

Figure out how to initialize viaduct for nimbus #3592

data-sync-user opened this issue Sep 30, 2020 · 7 comments

Comments

@data-sync-user
Copy link
Collaborator

data-sync-user commented Sep 30, 2020

The Nimbus SDK crate uses viaduct for networking. For it to work on Android, the consuming app must initialize and configure a viaduct backend as part of application startup, which app-services consumers currently do using the rusthttp support component. Ideally this would "just work" for such consumers when they come to use the Nimbus SDK.

Problem: rusthttp is part of the appservices megazord, which means that it configures the instance of viaduct that's bundled with the appservices megazord. Nimbus-SDK ships as its own standalone binary, so it has a separate copy of viaduct that does not share this configuration.

There are a couple of ways we could go here:

  • Provide a way for the application to configure the http backend for nimbus, e.g. some sort of "Nimbus.setHttpClient()" call that is has to do on startup next to its current call to "RustHttpConfig.setClient".
    • This would keep nimbus standalone, but IMHO would be a bit weird for consumers.
    • I don't know what bad things might happen having two versions of viaduct loaded into the same application. Maybe nothing...?
  • Bundle Nimbus SDK into the app-services megazord, so that it shares the same instance of viaduct.
    • I don't honestly recall our reasons for not doing this, but I'm sure we had some.
  • Have Nimbus SDK dynamically link against the instance of viaduct that's in the appservices megazord.
    • This keeps Nimbus as a standalone thing, but would be of unknown technical complexity. Might be fun to try though!

I'm personally leaning towards "bundle Nimbus in the app-services Megazord" as the short-term solution here, but I don't feel like I understand the landscape in sufficient detail to be confident it's the right thing.

Thoughts?

┆Issue is synchronized with this Jira Task
┆Issue Number: SDK-35

@data-sync-user
Copy link
Collaborator Author

➤ Ryan Kelly commented:

I'm moving this into the current sprint, because it's going to block SYNC-1701. No point integrating it into the reference browser if it can't talk to the network!

@data-sync-user
Copy link
Collaborator Author

➤ Vlad Filippov commented:

@rfk some prior art on this (via Glean) in https://github.com/a-s-dev/android-components/pull/1/files#diff-e2961bf02ab728cb8ac629829562cadeR43

@data-sync-user
Copy link
Collaborator Author

➤ Ryan Kelly commented:

Great, thanks for the context! Poking at this a bit:

Provide a way for the application to configure the http backend for nimbus, e.g. some sort of "Nimbus.setHttpClient()" call

This is the option pursued by the prototype above. The important part is in this PR:

a-s-dev/glean#1

Where we essentially copy the RustHttpConfig Kotlin code from app-services into glean, so that it can call LibGleanFFI.INSTANCE.viaduct_initialize instead of LibViaduct.INSTANCE.viaduct_initialize (that is, so that it can initialize the copy of viaduct that is bundled with the experiments lib rather than the one that is bundled with appservices).

@data-sync-user
Copy link
Collaborator Author

➤ Ryan Kelly commented:

We discussed this in the sync team meeting today and made some decisions on next steps.

Short-term, let's focus on unblocking the integration into reference browser by taking the Nimbus.setHttpClient() approach. @mhammond is going to investigate the details, and the thing I linked above is probably a good starting point, but the broad strokes would be:

  • Continue to build libnimbus.so with its own separate copy of viaduct

  • Copy the RustHttpConfig Kotlin code from Viaduct over into the Nimbus SDK repo, and tweak it to load functions from libnimbus.so instead.

  • There may be some fiddly build steps to integrate this hand-written code with the code generated by uniffi, but hopefully it won't be too bad...

  • Have the nimbus android-component wrap and expose setHttpClient method for the application to call as part of startup.

This should get us unblocked for the first milestone, but it doesn't seem like a good approach for the long-term.

In parallel with the above, let's revisit the discussion about how we will megaozord appservices, nimbus and glean for shipping in production in Fenix. This seems like the sort of thing that's best started in a google doc and perhaps a synchronous meeting if possible - @travis79 who are the right subset of glean folks to engage for that discussion?

@data-sync-user
Copy link
Collaborator Author

➤ Ryan Kelly commented:

By the way, I haven't totally discounted this as a possible option:

Have Nimbus SDK dynamically link against the instance of viaduct that's in the appservices megazord.

But it may not be necessary depending on where we land with megazording.

@data-sync-user
Copy link
Collaborator Author

➤ Travis Long commented:

Thanks @rfk, from the Glean side of things, I think Jan-Erik would probably be our best bet since he is both the most knowledgeable about the build system and is now the tech lead for Glean. Mike may want to be involved as well, so maybe include him as optional to the discussion.

@data-sync-user
Copy link
Collaborator Author

➤ Janet Dragojevic commented:

Mark, Ryan and Travis are happy with megazord solution for viaduct.

To do:

  • add libraries for unit test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants