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

[VAN-126440] README added. #87

Merged
merged 8 commits into from
Sep 19, 2024
Merged

[VAN-126440] README added. #87

merged 8 commits into from
Sep 19, 2024

Conversation

Shmalikov
Copy link
Collaborator

Added README.md how to use Python-SDK.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
Note: This SDK supports Python 3.8 and above.

## Usage
To use the API, you need to configure the `ApiClient` with a `Configuration` object.
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
To use the API, you need to configure the `ApiClient` with a `Configuration` object.
To use the API, you need to provider the `ApiClient` with a `Configuration` object.

to avoid over-using configure

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed to to provide

README.md Outdated Show resolved Hide resolved
README.md Outdated
config = Configuration.from_dict(config_dict)
```

Also, you can explicitly set the configuration parameters. E.g. for 2-legged OAuth 2.0 authentication:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, two-legged/basic is the worst example for this as it requires putting password into code. I think instead we should say that the usage of this option is not recommended for that reason but instead it's provided for callers who want to sub-class it with their own Configuration constructor

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Replaced with 3-legged and added warning not to store sensitive information in envirnoment variables or in secret storage.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
aggr_query_dto = AggregationQueryExecutionDTO.from_json(headcount_json)

# Passing 'Accept' header to ApiClient
api_client = ApiClient(header_name='Accept', header_value='text/csv')
Copy link
Collaborator

Choose a reason for hiding this comment

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

So ApiClient doesn't support more than one header?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ApiClient.set_default_header - method added in example.
Using it could be added multiple headers.

README.md Outdated
This Python SDK handles exceptions using custom exception classes derived from `OpenApiException`. Below are the main exception classes and their usage:

- `OpenApiException`: The base exception class for all API exceptions.
- `ApiTypeError`: Raised for type errors.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider adding just a bit more detail what type, value, attribute and key errors actually are. Don't need a novel, just a bit more than this

Copy link
Collaborator Author

@Shmalikov Shmalikov Sep 18, 2024

Choose a reason for hiding this comment

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

I found only a raising for ApiValueError and updated its comment.
Other exceptions were removed from README.md.
It's possible these exceptions were obsolete or intended for extended standard templates.

README.md doc suggestions
Incorporated feedback
@Shmalikov Shmalikov merged commit e704441 into main Sep 19, 2024
@Shmalikov Shmalikov deleted the feature/VAN-126440-readme branch September 25, 2024 23:18
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.

3 participants