-
Notifications
You must be signed in to change notification settings - Fork 225
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
docs: push end to end example #1741
Conversation
WalkthroughThe pull request includes significant updates to the documentation for integrating with Permify Cloud, specifically in the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
docs/mint.json (1)
Line range hint
48-48
: Update redirect rule for consistency.There's a redirect rule that points to the old quickstart page:
{ "source": "/setting-up/installation/overview", "destination": "/getting-started/quickstart" }This should be updated to point to the new end-to-end example page to maintain consistency and prevent broken redirects.
Apply this change:
{ "source": "/setting-up/installation/overview", - "destination": "/getting-started/quickstart" + "destination": "/getting-started/end-to-end-example" }docs/getting-started/end-to-end-example.mdx (3)
672-673
: Add input validation for entity IDs.The permission check examples should validate entity IDs before making the API call to prevent potential issues with invalid IDs.
Consider adding validation, for example in Python:
+if not repository_id or not repository_id.strip(): + raise ValueError("Repository ID cannot be empty") + body = permify.CheckBody( metadata=metadata, - entity=permify.Entity(id="repository1", type="repository"), + entity=permify.Entity(id=repository_id, type="repository"), permission="edit", subject=permify.Subject(id="user1", type="user") )Also applies to: 699-703, 722-723, 754-755, 784-785
254-257
: Consider adding error scenarios in examples.The example response shows only the successful case. It would be helpful to include examples of common error scenarios and how to handle them.
Add examples for cases like:
- Invalid tenant ID
- Missing permissions
- Schema validation errors
4-4
: Consider adding version compatibility information.The documentation should specify which versions of Permify Cloud are compatible with the examples provided.
Add a note about version compatibility at the beginning of the document:
title: 'End to End Example' +version_compatibility: 'Permify Cloud v1.x and above' mode: wide
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
docs/getting-started/end-to-end-example.mdx
(14 hunks)docs/mint.json
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/getting-started/end-to-end-example.mdx
[duplication] ~64-~64: Possible typo: you repeated a word
Context: ...t org.permify.model.TenantListRequest; ApiClient apiClient = new ApiClient(); apiClient.setBasePat...
(ENGLISH_WORD_REPEAT_RULE)
🔇 Additional comments (1)
docs/getting-started/end-to-end-example.mdx (1)
451-454
: LGTM: Clear visual documentation.
The schema section screenshot effectively illustrates the configuration history and provides clear visual context for users.
@@ -242,7 +242,7 @@ | |||
{ | |||
"group": "Getting Started", | |||
"pages": [ | |||
"getting-started/quickstart", | |||
"getting-started/end-to-end-example", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Update redirect rule to match new documentation structure
The redirect rule at line 48 points to /getting-started/quickstart
, but this page is being replaced with /getting-started/end-to-end-example
. This will create a broken redirect chain. Please update the redirect destination to match the new documentation structure.
- In
docs/mint.json
, update the redirect rule:
{
"source": "/setting-up/installation/overview",
"destination": "/getting-started/end-to-end-example"
}
🔗 Analysis chain
LGTM! Navigation structure update looks good.
The replacement of quickstart with end-to-end example in the "Getting Started" section aligns well with the PR objective of providing comprehensive documentation.
Let's verify that this page exists and is accessible:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the end-to-end example file exists
fd -t f "end-to-end-example.mdx" docs/getting-started/
Length of output: 98
Script:
#!/bin/bash
# Let's verify the redirect rule mentioned in the review comment
rg -A1 "/setting-up/installation/overview" docs/mint.json
Length of output: 161
<Tabs> | ||
<Tab title="Java"> | ||
```java | ||
import org.permify.ApiClient; | ||
import org.permify.api.TenancyApi; | ||
import org.permify.model.TenantListRequest; | ||
|
||
ApiClient apiClient = new ApiClient(); | ||
apiClient.setBasePath("<your-permify-endpoint:3476>"); | ||
apiClient.addDefaultHeader("Authorization", "Bearer <your-permify-api-key>"); | ||
``` | ||
</Tab> | ||
|
||
<Tab title="Python"> | ||
|
||
```python | ||
# Rest SDK for Python | ||
import permify | ||
|
||
configuration = permify.Configuration( | ||
host = "<your-permify-endpoint>:3476" | ||
) | ||
|
||
api_client = permify.ApiClient(configuration, | ||
header_name="Authorization", | ||
header_value="Bearer <your-permify-api-key>") | ||
``` | ||
</Tab> | ||
|
||
<Tab title="Javascript"> | ||
|
||
```javascript | ||
// Rest SDK for Javascript | ||
const permify = require('permify-javascript'); | ||
const apiClient = new permify.ApiClient("<your-permify-endpoint>:3476"); | ||
apiClient.defaultHeaders = {'Authorization': "Bearer <your-permify-api-key>"}; | ||
``` | ||
</Tab> | ||
|
||
<Tab title="Typescript"> | ||
|
||
```typescript | ||
// Rest SDK for Typescript | ||
import * as permify from 'permify-typescript'; | ||
const apiClient = new permify.ApiClient("<your-permify-endpoint:3476>"); | ||
apiClient.defaultHeaders = {'Authorization': "Bearer <your-permify-api-key>"}; | ||
``` | ||
</Tab> | ||
|
||
<Tab title="Node"> | ||
```javascript | ||
// gRPC SDK for Node | ||
const permify = require("@permify/permify-node"); | ||
|
||
const interceptor = permify.grpc.newAccessTokenInterceptor("<your-permify-api-key>"); | ||
const client = permify.grpc.newClient({ | ||
endpoint: "<your-permify-endpoint:3478>", | ||
cert: undefined, | ||
pk: undefined, | ||
certChain: undefined, | ||
insecure: false | ||
}, interceptor); | ||
``` | ||
</Tab> | ||
</Tabs> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve code snippet organization and error handling.
The client setup code snippets could be enhanced in the following ways:
- Add consistent error handling across all language examples
- Include comments about configuration options
- Add type safety checks where applicable
For the Node.js example, consider adding error handling for the configuration:
const client = permify.grpc.newClient({
endpoint: "<your-permify-endpoint:3478>",
cert: undefined,
pk: undefined,
certChain: undefined,
insecure: false
-}, interceptor);
+}, interceptor)
+ .catch(error => {
+ console.error('Failed to initialize client:', error);
+ throw error;
+ });
Committable suggestion was skipped due to low confidence.
🧰 Tools
🪛 LanguageTool
[duplication] ~64-~64: Possible typo: you repeated a word
Context: ...t org.permify.model.TenantListRequest; ApiClient apiClient = new ApiClient(); apiClient.setBasePat...
(ENGLISH_WORD_REPEAT_RULE)
docs: push end to end example
Summary by CodeRabbit