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

Remove exec calls from Elasticsearch instrument #348

Merged
merged 2 commits into from
Nov 4, 2019
Merged

Conversation

adamchainz
Copy link
Contributor

Part of #342.

Also tidy up the import pattern, smiilar to Redis changes in #341, and fix index name tracking when not passed as a keyword argument.

The speed difference is smaller than I thought.

Before:

In [2]: %timeit Instrument().install(); Instrument().uninstall()
95.5 µs ± 2.07 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)

After:

In [2]: %timeit Instrument().install(); Instrument().uninstall()
89.5 µs ± 674 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)

However the other benefits still stand - code being linted, etc.

Part of #342.

Also tidy up the import pattern, smiilar to Redis changes in #341, and fix index name tracking when not passed as a keyword argument.

The speed difference is smaller than I thought.

Before:

```
In [2]: %timeit Instrument().install(); Instrument().uninstall()
95.5 µs ± 2.07 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)
```

After:
```
In [2]: %timeit Instrument().install(); Instrument().uninstall()
89.5 µs ± 674 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)
```

However the other benefits still stand - code being linted, etc.
@adamchainz adamchainz merged commit d58c890 into master Nov 4, 2019
@adamchainz adamchainz deleted the issue_342_execs branch November 4, 2019 16:23
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.

1 participant