Add common jib sync code for processing syncmap #3489
Conversation
loosebazooka
requested review from
balopat,
briandealwis,
chanseokoh,
dgageot,
nkubala,
TadCordle and
tejal29
as code owners
Codecov Report
|
loosebazooka
force-pushed
the
jib-sync-parts-2
branch
2 times, most recently
from
0daea14
to
df50544
Compare
loosebazooka
changed the title
| Dest string `json:"dest"` | ||
| } | ||
|
|
||
| func getSyncMapFromSystem(cmd *exec.Cmd) (*SyncMap, error) { |
There was a problem hiding this comment.
maybe just getSyncMap? in the end we are under jib anyways
There was a problem hiding this comment.
there is actually another getSyncMap in this series of PRs, I guess since I split them out, this naming is a little weird.
There was a problem hiding this comment.
What does the other getSyncMap do?
There was a problem hiding this comment.
It checks if gradle/maven and then collects the right cmd and then calls out to this.
pkg/skaffold/build/jib/sync.go
Outdated
| return nil, errors.New("failed to get Jib Sync data") | ||
| } | ||
|
|
||
| line := bytes.Replace(matches[1], []byte(`\`), []byte(`\\`), -1) |
There was a problem hiding this comment.
What is this for? Windows support? Also don't see it covered in testcases unless I'm missing something.
There was a problem hiding this comment.
Good questions, this is copied from another jib-json section, I'll verify what it's doing.
There was a problem hiding this comment.
@chanseokoh great minds...
It's not going to be easy to cover this with integration tests as we don't have Windows integration tests just yet. We could (and should:)) take the output of jib on a Windows env and paste it in our testcases, similarly how we put unix outputs in there.
There was a problem hiding this comment.
yeah working on that, might need to restructure a few things to actually allow it.
There was a problem hiding this comment.
I dug into this some more. I think it's an artifact from previous jib outputs, it just happened to work because multiple back slashes are ignored. I think the reality is that we don't need to do this, I have a new commit that removes it, will push tomorrow.
Converts json output from jib maven/gradle command to a syncmap that can be processed within skaffold
loosebazooka
force-pushed
the
jib-sync-parts-2
branch
from
8bd5cfb
to
b599e1c
Compare
Introduces code to initialize sync caches, update them and checkfor differences when watched files change
It does not allow actually configuring sync yet. It just sets up the necessary underlying code to make calls out to jib and process the diffsConverted this into two separate PRs
This one now just calls out to jib to get the syncmap and coverts json to a usable object.