-
-
Notifications
You must be signed in to change notification settings - Fork 58
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
chore(example): fix example #265
base: main
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 12947462764Details
💛 - Coveralls |
headers = {"Prefer": "return=representation"} | ||
|
||
# does not match filter and will therefore only be received by the * listen, but not the INSERT listen | ||
requests.post( |
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.
Am I correct in thinking the requests
library is used here purely for testing purpose? if yes then I don't think this should be in the example as users will blindly copy this without understanding what is going on. Rather instructions should be left in a README on how to run the example and call the endpoints from a HTTP GUI client or CURL.
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.
Yes, @silentworks, the example is only for testing purposes. It is there to have a quick app to run and test the real-time implementation, actually. But thinking more about that, maybe it is better to remove this example and use an integration test instead.
What do you think?
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.
Yeah if its for testing purpose only then it should be in the tests directory as an integration test instead. This is how we've structured the postgrest-py
repo, it has normal unit tests and integration tests inside of the tests directory.
What kind of change does this PR introduce?
Bug fix, feature, docs update, ...
What is the current behavior?
Please link any relevant issues here.
What is the new behavior?
Feel free to include screenshots if it includes visual changes.
Additional context
Add any other context or screenshots.