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

Introspection. #1949

Closed
wants to merge 1 commit into from
Closed

Conversation

tomaszdrozdz
Copy link
Contributor

I think that kind of "tools" would be nice to have in core functionality.
Now two simple, but maybe some day we can have more.
If this is bad idea to have such tools in core functionality please share with me why.

@@ -0,0 +1,13 @@
async def get_uri(app):
Copy link
Member

Choose a reason for hiding this comment

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

Why is this fn named get_uri (singular) but returns 'uris' (plural)?

@@ -0,0 +1,13 @@
async def get_uri(app):
return {'uris': app.router.routes_all}
Copy link
Member

Choose a reason for hiding this comment

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

Why return a dict? Shouldn't it be return app.router.routes_all?

'named_request': app.named_request_middleware,
'named_response': app.named_response_middleware }

return {'middlewares': middlewares}
Copy link
Member

Choose a reason for hiding this comment

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

Why return a dict? Shouldn't it be return middlewares?

@ashleysommer
Copy link
Member

Hi @tomaszdrozdz
Thanks for this PR. I've left some in-line comments above.

Overall, I'm struggling to see how this would be useful or helpful.
Why would a user want to do this:

from sanic import introspection
uris = introspection.get_uri(app)

when you can already simply do:

uris = app.router.routes_all

The second option is faster to execute, one fewer lines, and easier to read and understand.

Similarly, why would a user want to do:

from sanic import introspection
request_middlewares = introspection.get_middlewares(app)['request']

when you can already simply do:

request_middlewares = app.request_middleware

Again, thats faster to execute, fewer lines, and easier to read and understand.

@tomaszdrozdz
Copy link
Contributor Author

tomaszdrozdz commented Oct 6, 2020

Thank You for comments.

Your comments about returning dicts, ... are right.
I Just wanted to get opinion about this idea, and if You like it then implement it, but if You have reasons not to then here why not.
It had to dig a bit in code to know how to get this info so I thought we can have kind of "introspection" tools that will provide informations about Sanic inner state.
I know these are simple examples but we can have more difficult in future possibly ?

Or maybe in this case adding documentation that would explain how to do it would be enough ?

@tomaszdrozdz
Copy link
Contributor Author

tomaszdrozdz commented Oct 6, 2020

And off course for end user it would be as easy as:

app.get_uris()
app.get_middlewares()

Just what do You think about this idea, or do You prefer perhaps some documentation section like "introspection" ?

@ahopkins
Copy link
Member

ahopkins commented Oct 7, 2020

It sounds more like a documentation issue.

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