Skip to content

Tornado framework #323

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

Closed
wants to merge 19 commits into from
Closed

Conversation

laerteallan
Copy link

Added integration with tornado framework.
Added the file "tornado.asciidoc" in docs directory . The file "tornado.ascii" is like the file "flask.asciidoc".

Laerte Allan de Oliveira Araujo added 13 commits October 14, 2018 00:18
Was added commit initial of integration
framework tornado.
was added file with utils functions
Was refactored of name class and in function
Was added unittests
Was added file configuration of tornado same
with flask
Was added paras from async process.

Added duration from async process.
Was adde unit test to framework tornado
Was added test for codes changes
Was moved the httresponse
Was added a new fucntion to parser url
Was added the version from a version of
installed tornado
Was added the version from a version of
installed tornado
@elasticmachine
Copy link

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@ghost
Copy link

ghost commented Nov 13, 2018

Hi @laerteallan, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in yout Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?

@beniwohli
Copy link
Contributor

Hi @laerteallan. This is great work! Thank you very much! I hope to have time to take a closer look some time this week.

@nicholasamorim
Copy link

Hey, any news on this? Can I help somehow?

@laerteallan
Copy link
Author

Hey, any news on this? Can I help somehow?

I can. What you need?

@nicholasamorim
Copy link

Well, help to put the work on it to get this merged to master. I reckon the official version still has no Tornado support, right?

@laerteallan
Copy link
Author

laerteallan commented Apr 13, 2019

Well, help to put the work on it to get this merged to master. I reckon the official version still has no Tornado support, right?

Yes still is'nt. But I'm using this version tornado_elastic in my company and of the project that are runing in production.

I created a package of installation for someone that want to use.

@basepi
Copy link
Contributor

basepi commented Jan 18, 2020

@laerteallan We really appreciate the work you did here. I completed a different implementation in #661 and wanted to explain why we went that direction instead of this direction.

We're constantly looking for ways to remove friction from the onboarding experience. Fewer code changes in onboarding make it an easier sell for users. Implementing our own RequestHandler subclass as you have is the most obvious way to proceed. Unfortunately, it has two pretty severe drawbacks for onboarding:

  1. The user has to replace all instances of RequestHandler in their code with our special APMRequestHandler, increasing the amount of code change required to use the tool.
  2. The functions on_finish and prepare are designed to be overridden by the user, for any number of purposes. This means the user would also have to remember to call super() in order to retain our behavior in those functions.

I was stuck for a long time on how to solve these problems. Then I realized that I could instrument RequestHandler._execute instead! It's not as clean as I would like, but it was the only way I could see to maintain an easy onboarding experience. (Really I just wish Tornado had an established middleware or signal mechanism, like Flask and Django 🤦‍♂ )

Anyway, you can see the result of my work in #661. Comments are welcome. And thank you again for the contribution, and our apologies that we weren't able to accept it!

@basepi basepi closed this Jan 18, 2020
@zube zube bot removed the [zube]: Done label Oct 13, 2020
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.

6 participants