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

Allow for a custom MetricsCollector class to be loaded at runtime #3146

Merged

Conversation

nipper
Copy link
Contributor

@nipper nipper commented Feb 18, 2022

Description

This adds in the ability to load a custom subclass of MetricsCollector at startup. This is so that if you want to have a private implementation of MetricsCollector you can.

It does this by adding the value of -1 to MetricsCollectors which represents Custom and an optional metrics_custom_import that is an optional string in the form of module.sub_module.ClassName. When both are provided it imports the class and returns an instance of it.

Motivation and Context

I need to have a private custom metrics collector that I'd like to use and this provides a backwards compatible way to do it.

Have you tested this? If so, how?

I've included a new unit test: test_custom_metrics_collector in test/scheduler_test.py

Since checks are blocked w/o maintainer approval I ran them here: https://github.com/nipper/luigi/pull/1/checks

@nipper nipper requested review from dlstadther and a team as code owners February 18, 2022 01:53
@nipper nipper force-pushed the change-config-to-allow-for-custom-metrics branch from 1701b20 to 10bff37 Compare February 18, 2022 12:39
Copy link
Collaborator

@dlstadther dlstadther left a comment

Choose a reason for hiding this comment

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

Nice and clever! Sorry for such delay in review and merge

@dlstadther dlstadther merged commit d9450d4 into spotify:master Mar 12, 2022
@nipper
Copy link
Contributor Author

nipper commented Mar 12, 2022

Thanks! No worries!

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.

None yet

2 participants