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

Cleaner syntax for linking event schemas to event methods #57

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Zsailer
Copy link
Member

@Zsailer Zsailer commented Oct 9, 2020

This is a syntax sugar. It adds a new decorator, event_schema, that can be called above a method to describe what telemetry data is collected by that method. This cleanly separates the telemetry definition logic from the application logic.

Before:

    def method_to_record(self):
        ...
        self.eventlog.record_event(
            schema_name="my_event",
            version=1,
            event=...
        )

After this PR:

    @event_schema(
        schema_name="my_event",
        version=1
    )
    def method_to_record(self):
        ...
        self.eventlog.record_event(event=...)

Normally, I don't think this kind of syntax sugar is necessary. In this case, though, it makes the code more readable. You can quickly scan through multiple methods to see which methods emit telemetry data and what data each method emits. No need to sift through the source code of the methods themselves.

This is fully backwards compatible. It's merely for readability.

@Zsailer
Copy link
Member Author

Zsailer commented Apr 28, 2021

@kiendang, what do you think of this PR? (ignoring the linting errors for now 😆 )

@kiendang
Copy link
Member

I like the syntax. Currently waiting until the jupyter_server PR get merged so we can test this with the events over there.

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