Skip to content

Documentation for customizing snap-in configuration page #63

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

Merged
merged 15 commits into from
Aug 14, 2024

Conversation

codeon
Copy link
Contributor

@codeon codeon commented Jun 19, 2024

ISS-98485
ISS-100260
ISS-98417

@codeon codeon marked this pull request as ready for review June 20, 2024 13:10
@codeon codeon requested a review from a team as a code owner June 20, 2024 13:10

DevRev Snap-ins platform allows developers to define custom configuration pages for their Snap-ins. This feature provides flexibility in designing user-friendly and intuitive configuration experiences tailored to the specific needs of the Snap-in.

## Why Customize Configuration Pages?
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this heading, it's unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed.

Comment on lines 24 to 32
functions:
- name: org_snap_in_configuration_handler
description: Handler for processing organization configuration options.
- name: user_snap_in_configuration_handler
description: Handler for processing user configuration options.
- name: config_initializer
description: Generates the initial configuration options for both organization and user.

configuration_handler:

Choose a reason for hiding this comment

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

Having separate functions for each of these is a bit expensive. Let's combine these into a single function in our example.

Comment on lines 159 to 175
"message": "Snap-in updated successfully",
"success": true
}
```

#### Error response

If an error occurs while updating the snap-in, the response has the following format:

- Status Code: 4xx or 5xx
- Content-Type: application/json

Response body:
```json
{
"data": {},
"message": "Failed to update the snap-in. Err: {error details}, Status: {status code}",

Choose a reason for hiding this comment

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

I think the message field would be removed if we make the API beta.


async updateSnapInGlobalVariable(payload: Record<string, any>): Promise<HTTPResponse> {
try {
await axios.post(`${this.endpoint}/internal/snap-ins.update`, payload, this.configs);

Choose a reason for hiding this comment

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

reminder to update this to use devrevSDK?

return this.updateSnapInGlobalVariable(payload);
}

async updateSnapInGlobalVariable(payload: Record<string, any>): Promise<HTTPResponse> {

Choose a reason for hiding this comment

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

nit: globalVariable as a term is not exposed to the external developers. Let's use inputs?


If an error occurs, it catches the error and returns an error response with an appropriate status code and an error message containing the error details.

Please keep in mind that this endpoint is currently in BETA, and its functionality or parameters may change in future updates.

Choose a reason for hiding this comment

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

Thinking out loud - Should we have some sort of subscribe to beta changes button in our documentation which can help us keep track of who all external parties which are using a beta feature? It can help us in the future when we need to track down who all are depending on this feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ideally we should be doing this based on the endpoint and our logging. But I can put a note here.

Choose a reason for hiding this comment

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

Are there any external developers which are waiting for the event-retry documentation to become public? My suggestion would be to keep this particular documentation internal and let our main snap-ins like Email, Slack etc. onboard to this mechanism first to confirm that everything works and is stable in PROD.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, removed retries doc for now since we are also revamping the backend approach.

config:
runtime:
max_retries: 5 # number of retries before failing the event
min_interval: 10 # interval in seconds between each retry

Choose a reason for hiding this comment

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

nit: Let's use a more realistic value

Suggested change
min_interval: 10 # interval in seconds between each retry
min_interval: 120 # interval in seconds between each retry


The DevRev snap-in platform handles errors based on the ordering guarantees of the snap-in function.

For snap-in functions with relaxed ordering, non-retryable errors are marked as failed, and the errored event is propagated to the DevRev platform for tracking. Retryable errors are automatically retried based on the specified retry configuration. If the maximum number of retries is exhausted, the errored events are moved to a dead-letter queue (DLQ) for further handling.

Choose a reason for hiding this comment

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

Right now all snap-in-functions have relaxed ordering. Mentioning this here might be confusing.

Suggested change
For snap-in functions with relaxed ordering, non-retryable errors are marked as failed, and the errored event is propagated to the DevRev platform for tracking. Retryable errors are automatically retried based on the specified retry configuration. If the maximum number of retries is exhausted, the errored events are moved to a dead-letter queue (DLQ) for further handling.
For snap-in functions with ordering, non-retryable errors are marked as failed, and the errored event is propagated to the DevRev platform for tracking. Retryable errors are automatically retried based on the specified retry configuration. If the maximum number of retries is exhausted, the errored events are moved to a dead-letter queue (DLQ) for further handling.

@codeon codeon changed the title Documentation for snap-in configuration page and event retries Documentation for customizing snap-in configuration page Aug 13, 2024
Copy link
Contributor

Copy link
Contributor

@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.

Some capitalization and styling suggestions but fundamentally ok.

Copy link
Contributor

@codeon codeon merged commit 651828d into main Aug 14, 2024
2 checks passed
@codeon codeon deleted the ISS-98485-ISS-100260-ISS-98417 branch August 14, 2024 06:16
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.

4 participants