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

Add ADaaS documentation. #64

Merged
merged 43 commits into from
Jul 24, 2024
Merged

Add ADaaS documentation. #64

merged 43 commits into from
Jul 24, 2024

Conversation

samod
Copy link
Contributor

@samod samod commented Jul 9, 2024

This PR provides ADaaS public documentation for 2P/3P developers.

app.devrev.ai/devrev/works/ISS-100391

@samod samod requested a review from a team as a code owner July 9, 2024 09:05
Copy link
Collaborator

@bc-devrev bc-devrev left a comment

Choose a reason for hiding this comment

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

I didn't mark every instance, but capitalization of titles and common nouns needs to be fixed throughout. Just because something is in the terminology list or is abbreviated doesn't mean it should be capitalized.

@@ -143,6 +143,42 @@ navigation:
- page: Developer keyrings
slug: developer-keyring
path: ../docs/pages/references/keyrings/developer_keyring.mdx
- section: ADaaS
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is really deeply nested. Is it the right place?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we move it one level up? @shashankcube ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have moved it up one level.

A Worker has the responsibility of maintaining its own state that persists between phases in a Sync Run,
as well as between Sync Runs.

```mermaid
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fern does not support Mermaid diagrams. You can go to mermaid.live and export a PNG for inclusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can Fern render SVGs?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe so.

fern/versions/public.yml Outdated Show resolved Hide resolved
fern/versions/public.yml Outdated Show resolved Hide resolved
fern/versions/public.yml Outdated Show resolved Hide resolved
Keyrings are a DevRev-specific mechanism for managing authentication for External Systems.
They are called Connections in the DevRev app.

They provide a secure way to store and manage credentials within your DevRev Snap-In.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
They provide a secure way to store and manage credentials within your DevRev Snap-In.
They provide a secure way to store and manage credentials within your DevRev snap-In.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are snap-ins always lowercase? Since it is a sort of a brand name for DevRev?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have modified cases for snap-ins.

@@ -0,0 +1,39 @@
# Attachment Extraction phase

For the Attachment Extraction phase of the import process,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
For the Attachment Extraction phase of the import process,
For the attachment extraction phase of the import process,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We had a lengthy discussion regarding capitalizing terms and well, the Airdrop team argues, that capitalizing names of processes, domain objects, ... and other domain entities would make the documentation more clear.

Can we cross-link everything with the Terminology section?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed the Terminology section.

fern/docs/pages/references/adaas/extracting_metadata.mdx Outdated Show resolved Hide resolved
gw -->> user: Import deleted
```

An Initial Import consists of the following phases:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
An Initial Import consists of the following phases:
An initial import consists of the following phases:


## Forward Sync

A Forward Sync is a sync from an [External System](#external-system) to DevRev.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
A Forward Sync is a sync from an [External System](#external-system) to DevRev.
A forward sync is a sync from an [external system](#external-system) to DevRev.

Copy link

@erazemk
Copy link
Contributor

erazemk commented Jul 15, 2024

Titles are added twice:
image

@samod samod requested a review from bc-devrev July 18, 2024 17:53
Copy link

@samod
Copy link
Contributor Author

samod commented Jul 18, 2024

Dear @bc-devrev , @erazemk , also @radovan-jorgic.

I have managed to come through all the suggestions and comments, and also updated and edited the metadata chapter. Could you please re-review?

@samod
Copy link
Contributor Author

samod commented Jul 18, 2024

And also, how can I generate a preview?


### Triggering event

Airdrop initiates the attachments delete phase when the Sync is deleted from DevRev.
Copy link
Contributor

@radovan-jorgic radovan-jorgic Jul 18, 2024

Choose a reason for hiding this comment

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

Didn't know this. Attachments deletion starts when import is deleted (and not visible through the UI)? Or should we say when data (and metadata) is deleted instead of when Sync is deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FIxed.


Once the Attachment Extraction phase is done, the snap-in must respond to Airdrop with a message with eventType `EXTRACTION_ATTACHMENTS_DONE`.

If Attachment Extraction failed, the snap-in must respond to Airdrop with a message with eventType `EXTRACTION_ATTACHMENTS_ERROR`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious if there is agreement regarding usage of word "eventType". Should we use "human language"? Something like "with event of type `EVENT_TYPE``?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perfect.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant "event of type EXTRACTION_ATTACHMENTS_DONE" and not "event of type EVENT_TYPE EXTRACTION_ATTACHMENTS_DONE".

Copy link
Contributor

Choose a reason for hiding this comment

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

So replace all "event of type EVENT_TYPE" with just "event of type" now.


Once the data extraction is done, the snap-in must respond to Airdrop with a message with eventType `EXTRACTION_DATA_DONE`.

If data extraction failed in any moment of extraction, the snap-in must respond to Airdrop with a message with eventType `EXTRACTION_DATA_ERROR`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not tested but I believe we save the latest successful state and retrieve it in next iteration if import failed and we retried it? If this is true, maybe we can write it down also - can be really helpful?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not understand what you mean. Please, explain.

Copy link
Contributor

Choose a reason for hiding this comment

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

If import fails, the latest state (before we emitted error) should be sent to snap-in in next iteration if retry (re-run) button is cliked on the UI. This is not mentioned anywhere.

Copy link

Copy link

Copy link

Copy link

Copy link

Comment on lines 1 to 2
_Airdrop_ is DevRev’s solution to migrate data. It allows our customers to bring in their existing data from
load data from external systems and, export data back to external systems, and keep data in sync between DevRev and the external systems.
Copy link
Contributor

@erazemk erazemk Jul 23, 2024

Choose a reason for hiding this comment

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

Very weird sentence, especially this part: It allows our customers to bring in their existing data from load data from external systems and

2. Install required tools and packages:
- [devrev-cli](https://developer.devrev.ai/snapin-development/references/cli-install) (version 4.7 or higher)
- [jq](https://stedolan.github.io/jq)
- [golang](https://go.dev/doc/install)
Copy link
Contributor

Choose a reason for hiding this comment

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

What is Go needed for?

3. Create a new repo from the ADaaS template repo:
On [https://github.com/devrev/adaas-template](https://github.com/devrev/adaas-template) use the dropdown button `Use this template` and
then `Create a new repository`
4. Copy `Makefile.variable.example` to `Makefile.variable` and fill in the required variables.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not really a problem with the documentation, but I still don't understand why we can't use the standard .env and .env.example, everybody knows and uses those but us.

Each sync run is comprised out of phases. Phases follow sequentially, and each can consist of one or more invocations
of the snap-in.

![ADaaS extraction phases](../../img/adaas/adaas-extraction-phases.svg)
Copy link
Contributor

Choose a reason for hiding this comment

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

@bc-devrev Is there a way we can dynamically adapt the color scheme of the diagrams, e.g. providing separate diagrams for light and dark mode? Mermaid supports such changes, so if there is no way to do it currently, can we at least push Fern to implement Mermaid markdown support, since we already pay them a lot for their platform?

fern/docs/pages/adaas/extraction-phases.mdx Outdated Show resolved Hide resolved
Comment on lines 1 to 2
In the external sync unit extraction phase, the extractor is expected to obtain a list of external sync units.
that it can extract with the provided credentials and send it to Airdrop in its response.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
In the external sync unit extraction phase, the extractor is expected to obtain a list of external sync units.
that it can extract with the provided credentials and send it to Airdrop in its response.
In the external sync unit extraction phase the extractor is expected to obtain a list of external sync units
that it can extract with the provided credentials, and send them to Airdrop in its response.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thx. I am keeping "it" since it is a list we are sending.

fern/docs/pages/adaas/external-sync-units-extraction.mdx Outdated Show resolved Hide resolved
fern/docs/pages/adaas/external-sync-units-extraction.mdx Outdated Show resolved Hide resolved
Comment on lines 1 to 2
During the delete phases, the snap-in may clean up its state or other side effects in the third party systems.
In the most common extraction use cases, there is nothing to do and snap-ins may reply with the completion message.
Copy link
Contributor

@erazemk erazemk Jul 23, 2024

Choose a reason for hiding this comment

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

Suggested change
During the delete phases, the snap-in may clean up its state or other side effects in the third party systems.
In the most common extraction use cases, there is nothing to do and snap-ins may reply with the completion message.
During the deletion phases, the snap-in may clean up its state or other side effects in the third party systems.
In the most common extraction use cases, there is nothing to do and snap-ins may reply with the completion message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

fern/docs/pages/adaas/data-attachments-deletion.mdx Outdated Show resolved Hide resolved
fern/docs/pages/adaas/data-attachments-deletion.mdx Outdated Show resolved Hide resolved
fern/docs/pages/adaas/data-attachments-deletion.mdx Outdated Show resolved Hide resolved
fern/docs/pages/adaas/data-attachments-deletion.mdx Outdated Show resolved Hide resolved
Copy link

Copy link

Copy link

@samod samod merged commit 7a14a70 into main Jul 24, 2024
4 checks passed
@samod samod deleted the ISS-100391-adaas-documentation branch July 24, 2024 13:58
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

Successfully merging this pull request may close these issues.

None yet

4 participants