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

Reduce 213 requires from each knife call #12409

Merged
merged 1 commit into from Jan 4, 2022
Merged

Conversation

tas50
Copy link
Contributor

@tas50 tas50 commented Dec 26, 2021

Right now for terrible reasons knife bootstrap gets initialized during every knife command due to sadness in knife-cloud. That wouldn't be nearly as bad if we were a bit more lazy in the bootstrap plugin.

  1. JSON compat is actually used in the options so it needs to be required outside deps method. This works right now because we're not lazy all over the place and this thing gets loaded in 20 places
  2. Net::SSH doesn't even appear to be used here. This plugin is all train now so we shouldn't have to require that.
  3. Erubis should only get called when we're rendering out the template

Before:

There were 642 requires
The total time spend requiring was: 1.167499999748543

After

There were 429 requires
The total time spend requiring was: 0.6953230001963675

Signed-off-by: Tim Smith tsmith@chef.io

Right now for terrible reasons knife bootstrap gets initialized during every knife command due to sadness in knife-cloud. That wouldn't be nearly as bad if we were a bit more lazy in the bootstrap plugin.

1. JSON compat is actually used in the options so it needs to be required outside deps method. This works right now because we're not lazy all over the place and this thing gets loaded in 20 places
2. Net::SSH doesn't even appear to be used here. This plugin is all train now so we shouldn't have to require that.
3. Erubis should only get called when we're rendering out the template

Before:

```
There were 642 requires
The total time spend requiring was: 1.167499999748543
```

After

```
There were 429 requires
The total time spend requiring was: 0.6953230001963675
```

Signed-off-by: Tim Smith <tsmith@chef.io>
@tas50 tas50 requested review from a team as code owners December 26, 2021 05:21
@lamont-granquist lamont-granquist merged commit 4836ab4 into main Jan 4, 2022
@lamont-granquist lamont-granquist deleted the faster_knife branch January 4, 2022 20:48
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