Create External Plugin that reads from disk or url #3114

blueridger · 2021-08-11T01:11:47Z

Description

This creates a new special plugin: the External plugin. This is a dynamic plugin that allows 3rd parties to rapidly pipe data into an instance.

The External plugin can be used multiple times, because it simply uses the PluginId pattern "external/X" where X can be any name (but preferably an agreed upon name between the 3rd-party software and the instance maintainer).

The External plugin loads its graph and optionally its declaration and identityProposals from either:

the plugin config folder on disk
- To use this method, simply place the files into the config/plugins/external/X folder.
a base url that statically serves the files
- To use this method, simply serve the files statically with cross-origin enabled in the same directory, and add a config.json file in the instance's config/plugins/external/X folder with form: { "graphUrl": "https://www.myhost.com/path/to/file/graph.json" }

Supported files for either method are:

graph.json/graph.json.gzip (required) - works whether or not it is compressed using our library
declaration.json (optional) - if omitted, a default declaration with minimal node/edge types is used, but also graphs don't have to adhere to the declaration if they don't desire to be configured using our Weight Configuration UI.
identityProposals.json (optional) - if omitted, no identities are proposed

Test Plan

local config graph

checkout gh-pages branch of our cred repo
copy output/graphs/sourcecred/github/graph.json.gzip to config/plugins/external/github2/graph.json.gzip
In sourcecred.json, remove "sourcecred/initiatives" and replace "sourcecred/github" with "external/github2"
scdev go --no-load
verify output/graphs/sourcecred/github2/graph.json.gzip exists

also ended up testing with a non-zipped graph.json

remote url graph

add config/plugins/external/github2/config.json containing {"baseUrl": "https://raw.githubusercontent.com/sourcecred/cred/gh-pages/output/graphs/sourcecred/github/"}
delete existing github2 graphs from config and output directories.
scdev graph
verify output/graphs/sourcecred/github2/graph.json.gzip exists

META-DREAMER · 2021-08-11T14:52:03Z

Excited to see this moving forward! Initial thoughts:

Are users of external plugins expected to generate their own graphs outside of the sourcecred runtime when the other plugin graphs are generated? I was imagining the external plugin wouldn't be a static JSON file but instead a JS file that default exports a class representing the plugin which then gets executed at runtime (e.g. by the load and graph commands). That way external plugin authors can implement arbitrary logic to generate the plugin declaration / graph object and don't need to manage a separate process to update the graph.json file.

blueridger · 2021-08-11T15:54:58Z

So really, there is nothing stopping those types of external plugins from being developed right now, they would just have to be registered in our bundledPlugins/bundledDeclarations files. The lift to allow an instance maintainer to import unregistered plugins is likely pretty simple, and I can take a look at knocking that out too this week.

This PR introduces some interesting possibilities:

The graph is the easiest plugin requirement to build using our library, and allowing the declaration and identityProposals to be wholly ignored can bump dev speed, especially with just a small amount of documentation / examples added.
One-and-done static graphs generated using an observable notebook or the like, which could be powerful for the airdrop use-case.
Language agnostic capability. Since the only real requirement is JSON, it opens the door for a determined Python etc dev to forego using our library and generate graphs in their native language, even potentially for 3rd party development of helper libraries in other languages.
Support for remote endpoints allows server devs to expose less data by doing graph calculation on-server and allows compatibility with distributed storage of graphs like on IPFS.
This enables "graph sharing" across instances without needing other people's private keys, especially if we add identityProposals and declarations to the graph CLI output. You just point to their statically hosted raw files on github.

META-DREAMER · 2021-08-12T02:00:44Z

@blueridger Makes sense, yea I think this is def a worthwhile primitive to have. I didn't think about the support for remote endpoints, that's actually very powerful and helps a ton for inter-org value graphs.

I think the key then becomes to make graph construction as easy as possible via our NPM package. I would say improving the dev experience around that / documenting an example implementation would be more useful than the runtime plugins.

blueridger · 2021-08-12T20:09:27Z

Hadn't manual tested the remote case yet. Found and fixed a bug and updated test plan. Seeking reapproval @HammadJ

significant changes made

blueridger · 2021-08-13T18:12:42Z

Realized, this plugin would be significantly more useful in the library if it could be constructed with a configuration. And also that the 3 files shouldn't need to have the same url root in the remote case. Made these changes. Seeking reapproval @HammadJ

topocount

I'm so excited you're exploring this! I've found some nits, but otherwise I'm feeling quite good about this.

One question I had: What are the implications of moving plugin declarations to async land? I knew it would have to happen eventually, but do we have to consider adding extra prompts to accommodate longer wait times here?

topocount · 2021-08-17T02:01:03Z

packages/sourcecred/src/api/bundledDeclarations.js

+export async function getPluginDeclaration(
+  pluginId: PluginId,
+  storage: DataStorage
+): Promise<PluginDeclaration> {
+  const mapping = {


I appreciate this refactoring

topocount · 2021-08-17T02:03:42Z

packages/sourcecred/src/api/bundledPlugins.js

+      pluginId,
+      storage: new DiskStorage(process.cwd()),
+    });
+  return null;


just curious: why not an implicit undefined return here?

will change, probably just a years-old habit from the 1 year in college I spent writing java lol.

topocount · 2021-08-17T02:07:27Z

packages/sourcecred/src/plugins/external/plugin.js

+  fromJSON as declarationFromJSON,
+} from "../../analysis/pluginDeclaration";
+import {join as pathJoin} from "path";
+import {type TaskReporter} from "../../util/taskReporter";


type should outside the curly brackets here, if only types are in the import.

accepted all

topocount · 2021-08-17T02:09:53Z

packages/sourcecred/src/plugins/external/plugin.js

+import {loadJson, loadJsonWithDefault} from "../../util/storage";
+import {merge as mergeWeights} from "../../core/weights";
+import {weightsForDeclaration} from "../../analysis/pluginDeclaration";
+import {type PluginId} from "../../api/pluginId";


topocount · 2021-08-17T02:10:02Z

packages/sourcecred/src/plugins/external/plugin.js

+  identityProposalsParser,
+} from "../../core/ledger/identityProposal";
+import * as Combo from "../../util/combo";
+import {type DataStorage} from "../../core/storage";


topocount · 2021-08-17T02:17:09Z

packages/sourcecred/src/plugins/external/plugin.js

+type ExternalPluginConfig = {|
+  +graphUrl: string,
+  +declarationUrl?: string,
+  +identityProposalsUrl?: string,
+|};


I'd like to see a URL parser created for these, but that can probably be a separate issue, or even a good first issue. I thought we had one, but I can't find it. It'd be really good to "fail fast" on something like this before any network requests are made.

so true. create an issue?

blueridger · 2021-08-19T02:25:33Z

What are the implications of moving plugin declarations to async land? I knew it would have to happen eventually, but do we have to consider adding extra prompts to accommodate longer wait times here?

Fair question. I don't anticipate it being a problem, since we actually don't parallelize promises very much in our code, and even over network, it should be a very short fetch time.

blueridger force-pushed the file-plugin branch from b80b3ae to 508fc7c Compare August 11, 2021 01:14

Create External Plugin that reads from disk or url

9ba7e51

blueridger force-pushed the file-plugin branch from 508fc7c to 9ba7e51 Compare August 11, 2021 01:21

blueridger requested review from META-DREAMER and topocount August 11, 2021 01:22

META-DREAMER previously approved these changes Aug 12, 2021

View reviewed changes

Fixed directory issue with network-based config

41fc92f

blueridger force-pushed the file-plugin branch from 6659153 to 41fc92f Compare August 12, 2021 20:08

Allow url for each file and construction with config

11321df

topocount approved these changes Aug 17, 2021

View reviewed changes

blueridger linked an issue Aug 18, 2021 that may be closed by this pull request

Create External Plugin that reads from Disk or URL #3133

Closed

nitfix

9c1fb3c

blueridger merged commit d4df10d into main Aug 19, 2021

blueridger deleted the file-plugin branch August 19, 2021 19:36

This was referenced Aug 21, 2021

Use combo parser instead of compat for PluginDeclaration #3153

Merged

Add ConstructorPlugin for ExternalPlugin testing #3154

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create External Plugin that reads from disk or url #3114

Create External Plugin that reads from disk or url #3114

blueridger commented Aug 11, 2021 •

edited

META-DREAMER commented Aug 11, 2021

blueridger commented Aug 11, 2021 •

edited

META-DREAMER commented Aug 12, 2021

blueridger commented Aug 12, 2021

blueridger commented Aug 13, 2021

topocount left a comment

topocount Aug 17, 2021

topocount Aug 17, 2021

blueridger Aug 19, 2021

topocount Aug 17, 2021

blueridger Aug 19, 2021

topocount Aug 17, 2021

topocount Aug 17, 2021

topocount Aug 17, 2021

blueridger Aug 19, 2021

blueridger commented Aug 19, 2021

Create External Plugin that reads from disk or url #3114

Create External Plugin that reads from disk or url #3114

Conversation

blueridger commented Aug 11, 2021 • edited

Description

Test Plan

local config graph

remote url graph

META-DREAMER commented Aug 11, 2021

blueridger commented Aug 11, 2021 • edited

META-DREAMER commented Aug 12, 2021

blueridger commented Aug 12, 2021

blueridger commented Aug 13, 2021

topocount left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blueridger commented Aug 19, 2021

blueridger commented Aug 11, 2021 •

edited

blueridger commented Aug 11, 2021 •

edited