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

Styling: README should import only the class and method instead of entire module #23

Closed
Tracked by #126
adriantam opened this issue Jun 9, 2023 · 3 comments
Closed
Tracked by #126
Assignees
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers styling Issues related to code styling/best practices

Comments

@adriantam
Copy link
Member

adriantam commented Jun 9, 2023

Currently, the (README file)[https://github.com/openfga/python-sdk/blob/main/README.md] suggests importing openfga_sdk as a whole. For example, we have import openfga_sdk.

This is contrary to best practice in that only the relevant classes/methods should be imported. Instead, we should do something like

from openfga_sdk import ClientConfiguration

This will also require updating the example so that it does not have the prefix openfga_sdk.

As an example,

import openfga_sdk
from openfga_sdk.client import OpenFgaClient


async def main():
    configuration = openfga_sdk.ClientConfiguration(
        api_scheme = OPENFGA_API_SCHEME, # optional, defaults to "https"
        api_host = OPENFGA_API_HOST, # required, define without the scheme (e.g. api.fga.example instead of https://api.fga.example)
        store_id = OPENFGA_STORE_ID, # optional, not needed when calling `CreateStore` or `ListStores`
        authorization_model_id = OPENFGA_AUTHORIZATION_MODEL_ID, # Optional, can be overridden per request
    )
    # Enter a context with an instance of the OpenFgaClient
    async with OpenFgaClient(configuration) as fga_client:
        api_response = await fga_client.read_authorization_models()
        await fga_client.close()

should become

from openfga_sdk import ClientConfiguration
from openfga_sdk.client import OpenFgaClient


async def main():
    configuration = ClientConfiguration(
        api_scheme = OPENFGA_API_SCHEME, # optional, defaults to "https"
        api_host = OPENFGA_API_HOST, # required, define without the scheme (e.g. api.fga.example instead of https://api.fga.example)
        store_id = OPENFGA_STORE_ID, # optional, not needed when calling `CreateStore` or `ListStores`
        authorization_model_id = OPENFGA_AUTHORIZATION_MODEL_ID, # Optional, can be overridden per request
    )
    # Enter a context with an instance of the OpenFgaClient
    async with OpenFgaClient(configuration) as fga_client:
        api_response = await fga_client.read_authorization_models()
        await fga_client.close()

Ideally, we will also fix the SDK generator so that newly generated python SDKs will have the corresponding changes. This is tracked as openfga/sdk-generator#126. However, if that is difficult to do, simply focusing on the Python SDK side https://github.com/openfga/python-sdk/blob/main/README.md will help us as well.

@JRudransh
Copy link
Contributor

JRudransh commented Jan 17, 2024

My PR #63 Covers this Issue

@JRudransh
Copy link
Contributor

You can assign this issue to me

@JRudransh
Copy link
Contributor

PR #63 Fixes this Issue

@rhamzeh rhamzeh closed this as completed Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers styling Issues related to code styling/best practices
Projects
None yet
Development

No branches or pull requests

3 participants