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

Update ParseModuleConfiguration to only parse changed file (*.tf) #1404

Merged
merged 10 commits into from
Sep 22, 2023
Merged

Update ParseModuleConfiguration to only parse changed file (*.tf) #1404

merged 10 commits into from
Sep 22, 2023

Conversation

jpogran
Copy link
Contributor

@jpogran jpogran commented Sep 13, 2023 •
edited
Loading

This modifies ParseModuleConfiguration to only parse changed file if file already parsed and this is a didChange request.

Previously ParseModuleConfiguration would reparse not only the changed file but also the module containing the changed file.

Now ParseModuleConfiguration detects if the file has previously been parsed and the request is from a didChange event. If both are true, then it only parses the changed file and skips re-parsing the entire module. If this is false, it falls back to the previous behavior of re-parsing the entire module including the changed file.

@jpogran jpogran self-assigned this Sep 13, 2023
@jpogran jpogran force-pushed the optimize_single_file_did_change branch from 01cad71 to 50fcdf4 Compare September 13, 2023 13:18
@dbanck dbanck linked an issue Sep 18, 2023 that may be closed by this pull request
Optimize the work we do on didChange #1339
Open
@jpogran jpogran force-pushed the optimize_single_file_did_change branch from a0ab82f to c207c65 Compare September 18, 2023 12:32
@jpogran jpogran marked this pull request as ready for review September 18, 2023 15:15
@jpogran jpogran requested a review from a team as a code owner September 18, 2023 15:15
@radeksimko radeksimko mentioned this pull request Sep 18, 2023
Merged
@jpogran jpogran changed the title Optimize didChange for single files Update ParseModuleConfiguration to only parse changed file Sep 18, 2023
jpogran and others added 4 commits September 19, 2023 10:13
@jpogran
Optimize didChange for single files
532b3f1 
- detect if file has already been parsed and rpc request is for didChange
- only process changed file if above is true
@jpogran
tests
fa62eb9
@jpogran
w
3956181
@radeksimko @jpogran
fix tests
d4170e1
@jpogran jpogran force-pushed the optimize_single_file_did_change branch from 31152fe to d4170e1 Compare September 19, 2023 14:14
@radeksimko radeksimko added enhancement New feature or request performance Gotta go fast labels Sep 19, 2023
@radeksimko radeksimko mentioned this pull request Sep 19, 2023
Merged
radeksimko
radeksimko approved these changes Sep 22, 2023
View reviewed changes
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

Mostly minor nitpicky suggestions, feel free to merge once applied.

internal/terraform/module/module_ops_test.go Outdated Show resolved Hide resolved
internal/terraform/module/module_ops_test.go Outdated Show resolved Hide resolved
internal/terraform/module/module_ops_test.go Outdated Show resolved Hide resolved
internal/terraform/module/module_ops_test.go Outdated Show resolved Hide resolved
@jpogran @radeksimko
Apply suggestions from code review
6755b7d 
Co-authored-by: Radek Simko <radek.simko@gmail.com>
@jpogran jpogran merged commit 10137ee into main Sep 22, 2023
20 checks passed
@jpogran jpogran deleted the optimize_single_file_did_change branch September 22, 2023 18:59
@jpogran jpogran mentioned this pull request Sep 22, 2023
Open
@radeksimko radeksimko changed the title Update ParseModuleConfiguration to only parse changed file Update ParseModuleConfiguration to only parse changed file (*.tf) Sep 25, 2023
@radeksimko radeksimko mentioned this pull request Sep 25, 2023
Closed
@jpogran jpogran mentioned this pull request Sep 26, 2023
Merged
jpogran added a commit that referenced this pull request Sep 26, 2023
@jpogran
Update ParseVariables for single files
334433f 
Modifies ParsedVarsFiles to only re-parse the single tfvars file which is being changed, if the job was scheduled as part of `textDocument/didChange` request.

This is a follow up to #1404 which updated the parsing job for terraform files.
jpogran added a commit that referenced this pull request Sep 27, 2023
@jpogran
Update ParseVariables for single files
f97a0d3 
Modifies ParsedVarsFiles to only re-parse the single tfvars file which is being changed, if the job was scheduled as part of `textDocument/didChange` request.

This is a follow up to #1404 which updated the parsing job for terraform files.
jpogran added a commit that referenced this pull request Sep 28, 2023
@jpogran
Update ParseVariables for single files
d9cae9e 
Modifies ParsedVarsFiles to only re-parse the single tfvars file which is being changed, if the job was scheduled as part of `textDocument/didChange` request.

This is a follow up to #1404 which updated the parsing job for terraform files.
jpogran added a commit that referenced this pull request Sep 28, 2023
@jpogran
Update ParseVariables for single files
4828c43 
Modifies ParsedVarsFiles to only re-parse the single tfvars file which is being changed, if the job was scheduled as part of `textDocument/didChange` request.

This is a follow up to #1404 which updated the parsing job for terraform files.
jpogran added a commit that referenced this pull request Sep 28, 2023
@jpogran
Update ParseVariables for single files
b2e2829 
Modifies ParsedVarsFiles to only re-parse the single tfvars file which is being changed, if the job was scheduled as part of `textDocument/didChange` request.

This is a follow up to #1404 which updated the parsing job for terraform files.
@radeksimko radeksimko mentioned this pull request Sep 29, 2023
Merged
jpogran added a commit that referenced this pull request Sep 29, 2023
@jpogran
Update ParseVariables for single files
9e1e96b 
Modifies ParsedVarsFiles to only re-parse the single tfvars file which is being changed, if the job was scheduled as part of `textDocument/didChange` request.

This is a follow up to #1404 which updated the parsing job for terraform files.
jpogran added a commit that referenced this pull request Oct 3, 2023
@jpogran
Update ParseVariables for single files (#1422)
449d442 
Modifies ParsedVarsFiles to only re-parse the single tfvars file which is being changed, if the job was scheduled as part of `textDocument/didChange` request.

This is a follow up to #1404 which updated the parsing job for terraform files.
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 26, 2023
@xiehan xiehan added this to the v0.32.0 (tentative) milestone Jan 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dbanck dbanck dbanck left review comments

@radeksimko radeksimko radeksimko approved these changes

Assignees

@jpogran jpogran

Labels
enhancement New feature or request performance Gotta go fast
Projects
None yet
Milestone
v0.32.0
Development

Successfully merging this pull request may close these issues.

Optimize the work we do on didChange
4 participants
@jpogran @dbanck @radeksimko @xiehan