Skip to content

Made priorities configurable #61

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

Merged
merged 1 commit into from
Jun 19, 2017

Conversation

rdohms
Copy link
Contributor

@rdohms rdohms commented Jun 15, 2017

In order to allow users to move the listeners in their priorities queue
the value is now configurable via configuration parameters. In order to
maintain BC these values default to 0 which is the current value.

this closes #49.

@rdohms
Copy link
Contributor Author

rdohms commented Jun 15, 2017

Worth noting, I added each parameter as a scalar node to avoid adding complexity to the Extension, but i'm happy to make it an array node and extend the extension if desired..

@rdohms rdohms force-pushed the configurable-priority branch from a2a5b00 to b39d528 Compare June 15, 2017 14:15
@rdohms
Copy link
Contributor Author

rdohms commented Jun 15, 2017

Fixed the extra line left behind, oops.

@Jean85
Copy link
Contributor

Jean85 commented Jun 15, 2017

First, thanks for the PR!
IMHO, an array with fixed keys would be more elegant. Something like this:

listener_priorities:
    request: 0
    kernel_exception: 0
    console_exception: 0

Is it too much complicated do be done?

@rdohms
Copy link
Contributor Author

rdohms commented Jun 16, 2017

@Jean85 should not be, i had done it initially. I'll add some more complexity to the extension but should be good. Gimme a sec.

@rdohms rdohms force-pushed the configurable-priority branch from b39d528 to 28fc2e6 Compare June 16, 2017 14:13
@rdohms
Copy link
Contributor Author

rdohms commented Jun 16, 2017

@Jean85 there you go. I tried to go with making the sentry.listener_priorities parameter an array and then using Symfony expressions in the services.yml file, but that has proven to not be supported in the priority section.

So i went with the simple approach of making each a scalar parameter, making config nice and clean and leaving the rest hidden behind the bundle wall.

Let me know if this needs cleaning up more, i decided to not exclude the sentry.listener_priorities, but for no strong reason at all.

In order to allow users to move the listeners in their priorities queue
the value is now configurable via configuration parameters. In order to
maintain BC these values default to 0 which is the current value.
@rdohms rdohms force-pushed the configurable-priority branch from 28fc2e6 to e8942b5 Compare June 16, 2017 14:24
@Jean85 Jean85 merged commit 325714a into getsentry:master Jun 19, 2017
@Jean85
Copy link
Contributor

Jean85 commented Jun 19, 2017

Thanks, it's perfect! 👍
I'll work on the release ASAP

@Jean85
Copy link
Contributor

Jean85 commented Jun 19, 2017

@rdohms
Copy link
Contributor Author

rdohms commented Jun 19, 2017

Thanks!

@Jean85 Jean85 modified the milestone: Stable release 1.0 Aug 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Listener priority on kernel.exception
2 participants