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

A more robust logic to parse API settings. #53

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

blacklight
Copy link

All key-value settings in api.py were parsed through the following syntax:

if settings["something"] is None:
  ...

This is problematic because it forces downstream implementations to also provide all the fields, also when they are not required.

In particular, when new fields are added, downstream implementations also have to provide them, or the integration will break.

The settings.get("something") syntax should be preferred, and settings itself should also be initialized to a dict by default to prevent dereferencing a None (I mean, if no settings are provided at all the code should probably still break, but with a relevant error instead of a fuzzier TypeError).

Closes: #51
Closes: #52
Closes: https://github.com/Pioverpie/privatebin-api#12

All key-value settings in api.py were parsed through the following syntax:

```python
if settings["something"] is None:
  ...
```

This is problematic because it forces downstream implementations to also
provide all the fields, also when they are not required.

In particular, when new fields are added, downstream implementations
also have to provide them, or the integration will break.

The `settings.get("something")` syntax should be preferred, and settings
itself should also be initialized to a dict by default to prevent
dereferencing a `None` (I mean, if no settings are provided at all the
code should probably still break, but with a relevant error instead of a
fuzzier `TypeError`).

Closes: r4sas#51
Closes: r4sas#52
Closes: https://github.com/Pioverpie/privatebin-api#12
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.

Make the API settings more flexible KeyError: "auth"
1 participant