Skip to content
This repository was archived by the owner on Aug 2, 2022. It is now read-only.

Added API Routes and Zeppelin Backend Adaptor #24

Merged
merged 6 commits into from
Aug 15, 2020

Conversation

ps48
Copy link
Contributor

@ps48 ps48 commented Aug 12, 2020

Issue #, if available:

Description of changes:

  1. Added API Routes for Notebooks and Paragraph
  2. Added Default Backend Placeholder
  3. Added Zeppelin Backend Aadaptor

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ps48 ps48 changed the title Added API Routes and Zeppelin Backend Aadaptor Added API Routes and Zeppelin Backend Adaptor Aug 12, 2020
@ps48 ps48 requested a review from chloe-zh August 12, 2020 05:20
@chloe-zh chloe-zh requested a review from davidcui1225 August 13, 2020 19:45
// Delete a paragraph
router.delete(
{
path: `${API_PREFIX}/para/{ids*2}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain this {ids*2}? I've never seen this syntax in an API route

Copy link
Contributor Author

@ps48 ps48 Aug 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From Hapi Docs: https://hapi.dev/tutorials/routing/?lang=en_US#-multi-segment-parameters
So, in short if you have multiple segments to a route e.g here /api/{noteId}/{paragraphId}, you can specify number of dynamic segments with a "number" (/api/{id*2})and then split id with a "/" to get noteId and paragraphId

Copy link
Contributor

@davidcui1225 davidcui1225 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some general comments: In my opinion, shortening Paragraph to Para introduces more confusion than the benefit from the shorter variable names- something I would consider. Mostly looks good though

@ps48 ps48 requested review from chloe-zh and davidcui1225 August 15, 2020 00:50
Copy link
Member

@chloe-zh chloe-zh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the delivering the changes and addressing all the comments!

@ps48 ps48 merged commit f7ccce9 into opendistro-for-elasticsearch:dev Aug 15, 2020
ps48 added a commit that referenced this pull request Aug 21, 2020
* Initial Commit - new platform, gitignore, license header (#18)

* initial commit with new platform structure

* adding gitignore

* adding Amazon header

* added .DS_Store in gitignore, added header in app.tsx (#23)

* Added API Routes and Zeppelin Backend Adaptor   (#24)

* added .DS_Store in gitignore, added header in app.tsx

* adding API routes and backend adaptor with Zeppelin

* changed para to paragraph for all parameters, simplified backend assignment

* modified error catches in Zeppelin Backend, modified API endpoints for paragraphs

* renamed functions and added parameter documentation

* propagating error message to UI

* added new route for visualizations and minor edits to noterouter (#25)

* added new RFC, supporting docs and images (#29)

* added dashboard dependency, added main, notebook and note_buttons component (#27)

* Added paragraph components and UI helpers (#28)

* added helpers and paragraph components

* added more documentation, minor refactors

* Updating docs and adding Zeppelin Patch PoC (#30)

* adding zeppelin patch to poc, moving docs, updating default scehma

* fixing typo in README

* removed cloning command, added comments for elasticsearch client configuration

* adding link to clone notebooks repo

* Updated workflow documentation, fixed link to POC (#33)

* updating screenshot, adding documentation for workflow/schema

* fixing link to POC

* updated docuemntation with usage, more images and example notebook

* updating schema doc and build link fix

* typo fix in API doc and usage doc
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants