feat(proxy): demo warp based REST API endpoints #250

xla · 2020-03-25T03:45:05Z

Alternative follow-up to #220, in response to the issues I see with pulling in Rocket as a dependency, which is at the core of the great prior work of @garbados in #226. Also just for brevity so we can compare our options as this approach uses our existing web stack based on warp.

To show an end-to-end integration I focused on the endpoint around projects as it forced me to handle CoCo and Registry equally as well as read and write operations. This includes serialisation of request inputs and response bodies, while trying to adhere to our strict separation of core domain and materialisation in one of our API. This approach is comparable to what we tried to achieve with our GraphQL API and the isolation of it in the graphql module.

Boostrap warp based REST stack
Introduce custom error infrastructure to translate core errors into JSON responses
Pass state down to filters and handlers
Implement project filters (create, get, list, register)
Implement project handlers (create, get, list, register)
Test end-to-end request chain

proxy/src/http/error.rs

proxy/src/http/project.rs

jxs · 2020-03-25T16:14:51Z

proxy/src/http/project.rs

+    }
+}
+
+impl Serialize for project::Project {


should Serialize impl'ed on crate::project instead of here?

I'm on the fence, on one side it's nice to use the derive macros. On the other hand there will be slight diversion between the core types and how they are represented over something like a JSON API. And that is really a concern of the API layer for us http. Happy to hear arguments in the other direction.

proxy/src/http/project.rs

proxy/src/project.rs

garbados · 2020-03-25T18:43:10Z

This looks like a good demo of how to do a REST API in Warp. Thanks @xla , this addresses the issues I ran into trying to do it myself. I've read through the implementation here and I'm prepared to move forward with it for the REST API over the Rocket implementation, echoing your concerns about dependency footprints.

rudolfs

Impressive change-set!! 🌟 🥇 🍰

From my perspective the proxy part could be merged if it can run in parallel to the existing graphql endpoints until we have a good migration strategy for the front-end.

The front-end still feels like it needs quite a lot of work to make the features that got migrated to the REST back-end work as they did before.

If we want to move forward, I'd recommend splitting it up into to PRs and merging the back-end and continue working on the front-end.

rudolfs · 2020-04-17T07:51:19Z

.buildkite/run.sh

 mkdir -p "$CACHE_FOLDER/npm"
 mkdir -p "$CACHE_FOLDER/yarn"
 mkdir -p "$CACHE_FOLDER/cargo"
 mkdir -p "$CACHE_FOLDER/rustup"

-
+export CYPRESS_CACHE_FOLDER="$CACHE_FOLDER/cypress"


I think this and L24 didn't make any difference to the CI build stability and can be removed again.

rudolfs · 2020-04-17T07:54:47Z

package.json

    "wait-on": "^4.0.1"
  },
  "scripts": {
-    "start": "run-p --race svelte:watch proxy:start graphiql:serve electron:start",
+    "start": "run-p --race svelte:watch proxy:start:test graphiql:serve electron:start",


I assume this will be reverted before merge?

rudolfs · 2020-04-17T07:55:04Z

package.json

@@ -87,7 +91,7 @@
    "svelte:watch": "yarn svelte:clean && rollup -c -w",
    "proxy:build": "cd proxy && cargo build --all-features --all-targets",
    "proxy:build:release": "cd proxy && cargo build --release --all-features --all-targets",
-    "proxy:start": "cd proxy && cargo run --release -- --registry=emulator",
+    "proxy:start": "cd proxy && cargo run -- --registry=emulator",


rudolfs · 2020-04-17T07:55:45Z

package.json

@@ -101,7 +105,7 @@
    "prettier": "prettier \"**/*.@(js|json|svelte|css|html)\" --ignore-path .gitignore",
    "prettier:check": "yarn prettier --check",
    "prettier:write": "yarn prettier --write",
-    "lint": "eslint . --ignore-path .gitignore --ext .js,.svelte"
+    "lint": "eslint . --ignore-path .gitignore --ext js,.svelte,.ts"


Is "js" vs ".js" a typo here?

rudolfs · 2020-04-17T07:59:38Z

proxy/API.md

+
+The examples provided use [curl](https://curl.haxx.se/) and the [window.fetch](https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch) function available in the JavaScript environments of modern browsers.
+
+### GET /projects


We should probably add a yarn script to start up the swagger live doc server and explain here how to use it and remove all the examples from this file.

rudolfs · 2020-04-17T08:00:53Z

proxy/Cargo.toml

@@ -30,7 +30,7 @@ tempfile = "3.1"
 tokio = { version = "0.2", features = [ "macros" ] }
 url17 = { package = "url", version = "1.7" }
 url = "2.1"
-warp = "0.2"
+warp = { git = "https://github.com/radicle-dev/warp", branch = "openapi", features = [ "openapi" ] }


Why do we need a fork for this? Will the openapi stuff get merged into mainline, so we don't have to maintain our own fork?

rudolfs · 2020-04-17T08:27:03Z

proxy/src/http/error.rs

+            "message".into(),
+            document::string()
+                .description("Human readable description of the error case")
+                .example("Malformed ID, supported charactes: [a-zA-Z0-9]"),


Suggested change

.example("Malformed ID, supported charactes: [a-zA-Z0-9]"),

.example("Malformed ID, supported characters: [a-zA-Z0-9]"),

rudolfs · 2020-04-17T08:42:04Z

ui/App.svelte

@@ -62,6 +63,7 @@
    }
  `;

+  // TODO(sos): dev mode in which you don't have to sign in on reload


This won't be necessary once identities are integrated at a protocol level in the backend. You'll create the identity once and it will persist across reboots.

MeBrei · 2020-04-16T14:04:24Z

proxy/src/http/identity.rs

+
+        state.end()
+    }
+}


Would the avatar related code be moved out of the file when implementing the rest of the queries (avatar query and later adding avatars to orgs), or would this part of the code be repeated?

MeBrei · 2020-04-16T14:38:21Z

ui/src/identity.ts

+    b: number;
+  };
+  emoji: string;
+}


Could we use the types from avatar.ts here to avoid duplication?

xla · 2020-04-20T12:18:45Z

Superseeded by #296

Follow-up to #250 * Implement project registration REST call * Implement register REST call * Remove outdated network query * Replace gql user endpoints with REST * Migrate to user REST endpoints * Migrate to control REST endpoints * Please le clippy * Remove outdated graphql docs * Fix typo * Remove last traces * Update proxy/src/lib.rs Co-Authored-By: Rūdolfs Ošiņš <[email protected]>

xla added 14 commits March 19, 2020 10:02

Bump proxy dependencies

fd37997

Attempt first end-to-end draft

633ba40

Move serialisation to http module

aa843ef

Merge branch 'master' into xla/220-warp-implementation

20f9943

Explore json response assertions

7b45e45

Pass state

a195ce7

Add docs

ef919df

Move project endpoints into own module

da71cc0

Flesh out error responses

695b7c9

Clean up

b9ffe18

Show case create endpoint

1f78617

Implement project registration

ac73fe1

Merge branch 'master' into xla/220-warp-implementation

dcad433

Test register

2546e50

xla added improvement labels Mar 25, 2020

xla requested review from cloudhead, rudolfs, garbados, sarahscott, NunoAlexandre and MeBrei March 25, 2020 03:45

xla self-assigned this Mar 25, 2020

xla added 3 commits March 25, 2020 05:10

Implement list endpoint

41ec0fa

Simplify test assertions

b7e9d0d

Fix reference

ee5fccf

jxs reviewed Mar 25, 2020

View reviewed changes

Merge branch 'master' into xla/220-warp-implementation

7a02e2f

xla and others added 17 commits April 16, 2020 01:35

Fix tests

4330574

sourceBrowserStore & parameterized FileSource

3948265

simplify RevisionSelector

500052a

update revision event

751e88e

simplifies FileList

9a97fa7

update source path

351fc9a

Implement first source browsing endpoint

cf6c842

Use string enum for object type documentation

ab3f0ed

Adjust types to match proxy

ca4423f

Make prettier check happy

06991c6

Correct tye missmatch

cece03b

Fix field

88e5854

Implement commit endpoint

17d6009

Implement tree endpoint

9dd2b79

Implement branches endpoint

d381d07

Implement tags endpoint

63d64e9

Fix docs

df138a0

rudolfs suggested changes Apr 17, 2020

View reviewed changes

MeBrei reviewed Apr 17, 2020

View reviewed changes

rudolfs mentioned this pull request Apr 17, 2020

feat: switch to REST API for most endpoints #293

Merged

xla and others added 3 commits April 17, 2020 22:04

Complete first circle of source browsing

07c8538

blob name and revision event fix

ebcc13b

Merge branch 'master' into xla/220-warp-implementation

f6c6fef

xla closed this Apr 20, 2020

xla mentioned this pull request Apr 27, 2020

refactor: Migrate outstanding endpoints to REST #312

Merged

rudolfs deleted the xla/220-warp-implementation branch May 12, 2020 16:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(proxy): demo warp based REST API endpoints #250

feat(proxy): demo warp based REST API endpoints #250

xla commented Mar 25, 2020

jxs Mar 25, 2020 •

edited

Loading

xla Mar 26, 2020

garbados commented Mar 25, 2020

rudolfs left a comment

rudolfs Apr 17, 2020

rudolfs Apr 17, 2020

rudolfs Apr 17, 2020

rudolfs Apr 17, 2020

rudolfs Apr 17, 2020

rudolfs Apr 17, 2020

rudolfs Apr 17, 2020

rudolfs Apr 17, 2020

MeBrei Apr 16, 2020

MeBrei Apr 16, 2020

xla commented Apr 20, 2020


		The examples provided use [curl](https://curl.haxx.se/) and the [window.fetch](https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch) function available in the JavaScript environments of modern browsers.

		### GET /projects

	.example("Malformed ID, supported charactes: [a-zA-Z0-9]"),
	.example("Malformed ID, supported characters: [a-zA-Z0-9]"),

feat(proxy): demo warp based REST API endpoints #250

feat(proxy): demo warp based REST API endpoints #250

Conversation

xla commented Mar 25, 2020

jxs Mar 25, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garbados commented Mar 25, 2020

rudolfs 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

Choose a reason for hiding this comment

xla commented Apr 20, 2020

jxs Mar 25, 2020 •

edited

Loading