Reset index when force pushed #3536

gorkem · 2020-07-09T16:34:59Z

What type of PR is this?

/kind bug

What does does this PR do / why we need it:
It resets the index for synced files to ensure that all files are synced when a pod is started/restarted or it is force pushed.

Which issue(s) this PR fixes:

Fixes #3479

How to test changes / Special notes to the reviewer:

Instructions are provided on #3479

openshift-ci-robot · 2020-07-09T16:35:24Z

Hi @gorkem. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

amitkrout · 2020-07-09T16:43:38Z

/ok-to-test

mik-dass · 2020-07-10T07:44:29Z

pkg/sync/adapter.go

@@ -76,6 +81,10 @@ func (a Adapter) SyncFiles(syncParameters common.SyncParameters) (isPushRequired
 			}
 		}

+		if isForcePush {
+			//reset the index
+			util.DeleteIndexFile(pushParameters.Path)


please handle the returned error

mik-dass · 2020-07-10T07:47:30Z

pkg/sync/adapter.go


 	// Sync source code to the component
 	// If syncing for the first time, sync the entire source directory
 	// If syncing to an already running component, sync the deltas
 	// If syncing from an odo watch process, skip this step, as we already have the list of changed and deleted files.
-	if !syncParameters.PodChanged && !pushParameters.ForceBuild && len(pushParameters.WatchFiles) == 0 && len(pushParameters.WatchDeletedFiles) == 0 {
+	if isWatch && !isForcePush {


please update the comments above regarding force pushing for watch

kadel · 2020-07-10T07:55:35Z

/retest

mik-dass · 2020-07-10T08:13:01Z

Can we include a integration test or a new issue for testing push against restarted pods?

girishramnani · 2020-07-15T08:06:11Z

/approve

openshift-ci-robot · 2020-07-15T08:06:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: girishramnani

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~~ [girishramnani]

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

mik-dass · 2020-07-15T08:49:44Z

pkg/sync/adapter.go

-			// apply the glob rules from the .gitignore/.odo file
-			// and ignore the files on which the rules apply and filter them out
-			filesChangedFiltered, filesDeletedFiltered := util.FilterIgnores(ret.FilesChanged, ret.FilesDeleted, absIgnoreRules)
+		// apply the glob rules from the .gitignore/.odo file


It should be.gitignore/.odoignore

addressed this in this PR #3614
if there is nothing else wrong them can you please lgtm @mik-dass ?

Updates syncfiles to reset the index to do a full snycronization when pod has changed, restarted ot doing a forcebuild.

codecov · 2020-07-21T18:40:44Z

Codecov Report

Merging #3536 into master will increase coverage by 0.06%.
The diff coverage is 81.81%.

@@            Coverage Diff             @@
##           master    #3536      +/-   ##
==========================================
+ Coverage   45.49%   45.56%   +0.06%     
==========================================
  Files         115      115              
  Lines       11697    11699       +2     
==========================================
+ Hits         5322     5331       +9     
+ Misses       5857     5847      -10     
- Partials      518      521       +3

Impacted Files	Coverage Δ
pkg/sync/adapter.go	`80.99% <81.81%> (-1.37%)`	⬇️
pkg/sync/sync.go	`52.47% <0.00%> (+8.91%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c2c609...41a501c. Read the comment docs.

mik-dass

/lgtm

openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jul 9, 2020

openshift-ci-robot requested review from cdrage and mohammedzee1000 July 9, 2020 16:35

openshift-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. Required by Prow. label Jul 9, 2020

gorkem mentioned this pull request Jul 9, 2020

odo watch deletes all files #3479

Closed

openshift-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. Required by Prow. labels Jul 9, 2020

mik-dass reviewed Jul 10, 2020

View reviewed changes

gorkem force-pushed the force-push-reset branch 2 times, most recently from d79342b to 293063d Compare July 13, 2020 14:04

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Jul 15, 2020

mik-dass reviewed Jul 15, 2020

View reviewed changes

jgwest mentioned this pull request Jul 15, 2020

Watch command should better handle file synchronization when push command is invoked before/after the watch #3585

Closed

gorkem force-pushed the force-push-reset branch from 293063d to d758bcf Compare July 20, 2020 16:17

Reset index when force pushed

41a501c

Updates syncfiles to reset the index to do a full snycronization when pod has changed, restarted ot doing a forcebuild.

gorkem force-pushed the force-push-reset branch from d758bcf to 41a501c Compare July 21, 2020 16:23

mik-dass approved these changes Jul 22, 2020

View reviewed changes

openshift-ci-robot assigned mik-dass Jul 22, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jul 22, 2020

openshift-merge-robot merged commit a95eef9 into redhat-developer:master Jul 22, 2020

gorkem deleted the force-push-reset branch July 22, 2020 12:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reset index when force pushed #3536

Reset index when force pushed #3536

gorkem commented Jul 9, 2020 •

edited

openshift-ci-robot commented Jul 9, 2020

amitkrout commented Jul 9, 2020

mik-dass Jul 10, 2020

mik-dass Jul 10, 2020 •

edited

kadel commented Jul 10, 2020

mik-dass commented Jul 10, 2020 •

edited

girishramnani commented Jul 15, 2020

openshift-ci-robot commented Jul 15, 2020

mik-dass Jul 15, 2020 •

edited

girishramnani Jul 21, 2020

mik-dass Jul 22, 2020

codecov bot commented Jul 21, 2020 •

edited

mik-dass left a comment

Reset index when force pushed #3536

Reset index when force pushed #3536

Conversation

gorkem commented Jul 9, 2020 • edited

openshift-ci-robot commented Jul 9, 2020

amitkrout commented Jul 9, 2020

mik-dass Jul 10, 2020

Choose a reason for hiding this comment

mik-dass Jul 10, 2020 • edited

Choose a reason for hiding this comment

kadel commented Jul 10, 2020

mik-dass commented Jul 10, 2020 • edited

girishramnani commented Jul 15, 2020

openshift-ci-robot commented Jul 15, 2020

mik-dass Jul 15, 2020 • edited

Choose a reason for hiding this comment

girishramnani Jul 21, 2020

Choose a reason for hiding this comment

mik-dass Jul 22, 2020

Choose a reason for hiding this comment

codecov bot commented Jul 21, 2020 • edited

Codecov Report

mik-dass left a comment

Choose a reason for hiding this comment

gorkem commented Jul 9, 2020 •

edited

mik-dass Jul 10, 2020 •

edited

mik-dass commented Jul 10, 2020 •

edited

mik-dass Jul 15, 2020 •

edited

codecov bot commented Jul 21, 2020 •

edited