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

Fixed unused endpoint key str #4

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

MyGodItsFull0fStars
Copy link
Collaborator

When registering API endpoints, the key was not used as the endpoint.
Instead, the model.__tablename__ value was used within the create_handler function and could not be changed by a user of LightAPI.

The problem I see with that approach was that while the example shows that endpoints can be defined using a dictionary:

app.register({"person": Person})
app.register({"custom-name": Company})

In this example, the endpoints person and company are defined with LightAPI as of now. The given endpoint key is ignored further on.

I added some logic that now uses the key, therefore the Company model is now using the endpoint custom-name.
Further on, the / at the beginning of the key string had to be omitted, otherwise the endpoint for Person would be ip-address:8000//person/.

Let me know if you have any questions/suggestions for this commit.

Note: The current implementation requires a Python version >= 3.10 because of https://peps.python.org/pep-0604/. If you envision a larger support with other Python versions, let me know, I can change the code but this will result in less readability.

@iklobato
Copy link
Owner

iklobato commented Sep 8, 2024

Hello @MyGodItsFull0fStars thanks for that! We can absolutely merge it, but can we add some unit tests for this?

@MyGodItsFull0fStars
Copy link
Collaborator Author

Hello @iklobato, oh yeah sure and good catch! I will add some unit tests to it before the merge.

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.

2 participants