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.

Sign up for GitHub

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

Jump to bottom

fix: performance of template loading #1189

Merged
merged 1 commit into from
Aug 25, 2022
Merged

fix: performance of template loading #1189

merged 1 commit into from
Aug 25, 2022

Conversation

matejvasek
Copy link
Contributor

@matejvasek matejvasek commented Aug 24, 2022 •
edited
Loading

Changes

  • 🐛 Optimise template loading: use locally checked out files, i.e. do not re-clone content from .git/ dir.

resolves: #1186

@matejvasek
fix: performance of template loading
334a232 
If repo is non-bare git repo treat is as plain FS template,
i.e. do not load it from ".git" date.

Signed-off-by: Matej Vasek <mvasek@redhat.com>
@knative-prow knative-prow bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 24, 2022
@matejvasek
Copy link
Contributor Author

matejvasek commented Aug 24, 2022 •
edited
Loading

Please try this out @sudhirverma this turns 40sec into 0.131sec.

@codecov
Copy link

codecov bot commented Aug 24, 2022

Codecov Report

Merging #1189 (334a232) into main (e6ec11b) will increase coverage by 0.03%.
The diff coverage is 82.35%.

@@            Coverage Diff             @@
##             main    #1189      +/-   ##
==========================================
+ Coverage   43.42%   43.45%   +0.03%     
==========================================
  Files          68       68              
  Lines        9606     9623      +17     
==========================================
+ Hits         4171     4182      +11     
- Misses       5038     5042       +4     
- Partials      397      399       +2
Impacted Files Coverage Δ
repository.go 72.42% <82.35%> (+0.59%) ⬆️
s2i/builder.go 61.04% <0.00%> (-1.21%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@matejvasek matejvasek linked an issue Aug 24, 2022 that may be closed by this pull request
Templates list command takes time to execute the command. #1186
Closed
@sudhirverma
Copy link

Please try this out @sudhirverma this turns 40sec into 0.131sec.

Thanks, @matejvasek now it execute the command fast :)

@matejvasek matejvasek mentioned this pull request Aug 25, 2022
Closed
zroubalik
zroubalik approved these changes Aug 25, 2022
View reviewed changes
Copy link
Contributor

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

/lgtm

/hold for others

@knative-prow knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 25, 2022
@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Aug 25, 2022
@knative-prow
Copy link

knative-prow bot commented Aug 25, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: matejvasek, zroubalik

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [matejvasek,zroubalik]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

lance
lance reviewed Aug 25, 2022
View reviewed changes
Copy link
Member

@lance lance left a comment

Choose a reason for hiding this comment

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

/lgtm

@matejvasek
Copy link
Contributor Author

/unhold

@knative-prow knative-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 25, 2022
@knative-prow knative-prow bot merged commit dca11da into knative:main Aug 25, 2022
@lance lance added this to the 0.26.0 Release milestone Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lance lance lance left review comments

@zroubalik zroubalik zroubalik approved these changes

@lkingland lkingland Awaiting requested review from lkingland

Assignees

@lance lance

@zroubalik zroubalik

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
0.26.0 Release
Development

Successfully merging this pull request may close these issues.

Templates list command takes time to execute the command.
4 participants
@matejvasek @sudhirverma @lance @zroubalik