Skip to content
This repository has been archived by the owner on Oct 14, 2020. It is now read-only.

[v5] Finalize v5 api #121

Closed
gregglind opened this issue Apr 2, 2018 · 4 comments
Closed

[v5] Finalize v5 api #121

gregglind opened this issue Apr 2, 2018 · 4 comments
Assignees
Labels

Comments

@gregglind
Copy link
Contributor

gregglind commented Apr 2, 2018

Yes

Maybe:

  • have cli to lint study config / jsonschema for such more tools.
  • tie and document with SG. (ask RRayborn)
  • catch addon startup errors using @Osmose error collector. (v3 of shield packet #55, point 13)
  • support queries for 'clients alive in study today'

NO:

Notes, unfinished.

glossary:

  • ssau shield-studies-addon-utils
  • ssat shield-studies-addon-template

shield-utils (browser.study.)

  • chooseBranch (weights, optional rng?)
  • configure // alternative is to new an object
    • shield or pioneer
    • name
    • weights?
    • variation?
    • ...
    • rules: pioneer, or isPioneer: true, or something!
  • permissions // STUDY IMPLEMENTER DON'T HAVE TO REMEMBER ALL THE PREFS
    • OK? "some boolean is summary for basic isEligble"
    • details
      • auto pb mode?
      • I can shield (my shield pref is true)
      • I can telemetry
  • telemetry (must handle pioneer as well)
    • send
    • configure?
    • setActive
    • unsetActive
  • userInfo?? // alternative is to get it from normandy modules
    • (needed? for survey urls, pings)
    • get
      • fx version
      • addons
      • ...
  • endStudy(...anEnding)
  • utils
    • surveyQueryArgs. => (get the args for survey?) openUrl? construct url?
  • sampling
    • hashing
    • unbalanced weighted selection
  • (proposed)
    • expiration
  • prefs
    • get
    • set

Example MVP Bootstrap.js

// alt extension prefs manager?
const studyConfig = {
    weights: 

}

studySetup.expired = await Study.hasExpired();  // per-session, but should be an observer / timer 

const theBranchPref = `shield.${addonWidgetId}.testingBranch`;
mybranch = await browser.shield.prefs.getStringPref(thePref) || await browser.shield.chooseVariation(weights);
studySetup.eligible = await Study.isEligible();
const { variation } = await browser.shieldUtils.info();
await browser.shield.configure(...studySetup)  // implies one study per addon

// handle install vs. regular run. 
await browser.shield.attemptRun()

await browser.shield.telemetry(aPing)
await browser.shield.endStudy(...anEnding, uninstall)
// endSTudy, ps has scary side effects, describe them.

NOT

// claim:  this seems non-idiomatic
const aStudy = browser.shield.createStudy(...options);
// benefit: cant accidentally call methods that need config

Big picture for template and tools

  • WebExtensionExperiment NOT Legacy addon
  • v5 for SSAU, SSAT -
  • GITHUB:
  • Linting Exists
  • USE THE MOZILLA INFRA for CI and testing if possible (except Github)
    • Unit Tests for sample study
    • CI on CircleCI (unit tests) / Task Cluster ? ???? (jmaher@)
      • SSAU
      • SSAT
    • TryServer
      • SSAT
  • merge with pioneer (what is that?)
    • Send Telemetry
    • Different methods for sample
  • Team: glind, bdanforth, pdh, motin, rhelmer, pdahiya, raymak, rdalal,
  • Does endStudy uninstall the addon? Probably
  • branch choice persistence preferred mechanism
    • restart, get same branch
    • testers can choose a branch (reasonably)
    • mechanism for branch is mostly not magic
      • chooseBranch(weights, unit test optional unityFractions) = R cut
@Osmose
Copy link

Osmose commented Apr 3, 2018

catch addon startup erros using @Osmose error collector. (#55, point 13)

There's not really anything you need to do for getting these collected; as long as they're errors that get logged to the browser console they'll get picked up (I was mixing up a different feature when I was saying you need to register something).

The bigger problem is that getting access and noticing your errors. Error collection is still just a prototype and will be removed from Firefox in Q2 or Q3. Access is limited to a subset of approved employees, and Sentry has nothing for setting up alerts for errors related only to your code. Also, errors are only collected from users on the Nightly channel and at a sample rate of 0.1% of total errors that occur. So unless your study is going to 100% of Nightly users and has a common error, the current prototype collection won't help you.

@gregglind
Copy link
Contributor Author

Proposed approach:

  1. Build this is in yaml, not json, for the sake of easy diff.
  2. Convert with npm install -g yamljs (yaml2json the file)

IN the PR, let's work on the yaml version, until we finalize.

I am open to deciding if the yaml is canonical, or the json.

gregglind added a commit that referenced this issue Apr 4, 2018
@gregglind
Copy link
Contributor Author

secondary note; I somehow borked / dont understand tracking branches and made it on MOzilla instead of Gregglind :)

@gregglind
Copy link
Contributor Author

installed / startup

  • alternative solution
    • during startup, have a localstorage with a 'installed' key
    • check if there
    • if not there, check eligible or die

This seems non idiomatic

Should 'configure' setActiveExperiment? I claim yes. Assumes all these ARE active experiments!

Claim: timers are best handled in the class not the utils directly.

motin pushed a commit that referenced this issue Apr 10, 2018
@motin motin changed the title finalize v5 api [v5] Finalize v5 api Apr 10, 2018
motin pushed a commit that referenced this issue Apr 10, 2018
motin pushed a commit that referenced this issue Apr 10, 2018
gregglind added a commit that referenced this issue Apr 12, 2018
gregglind added a commit that referenced this issue Apr 16, 2018
gregglind added a commit that referenced this issue Apr 20, 2018
Schemas for the experiment APIs - for review by all  Fix  #121
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants