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

Make it possible to disable auto-sync #4923

Merged
merged 6 commits into from
Oct 20, 2020

Conversation

briandealwis
Copy link
Member

Fixes: #4806

Description
Turn the sync.auto to a boolean.

User facing changes (remove if N/A)

  • auto-sync can now be disabled with sync.auto = false on the respective artifacts

@codecov
Copy link

codecov bot commented Oct 18, 2020

Codecov Report

Merging #4923 into master will increase coverage by 0.00%.
The diff coverage is 83.33%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4923   +/-   ##
=======================================
  Coverage   72.18%   72.19%           
=======================================
  Files         358      358           
  Lines       12402    12409    +7     
=======================================
+ Hits         8953     8959    +6     
- Misses       2795     2796    +1     
  Partials      654      654           
Impacted Files Coverage Δ
pkg/skaffold/schema/latest/config.go 100.00% <ø> (ø)
pkg/skaffold/schema/v2beta8/upgrade.go 88.88% <66.66%> (-11.12%) ⬇️
pkg/skaffold/build/buildpacks/env.go 87.50% <100.00%> (ø)
pkg/skaffold/schema/defaults/defaults.go 89.09% <100.00%> (ø)
pkg/skaffold/build/result.go 86.66% <0.00%> (+1.30%) ⬆️

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 5d8b798...a1b02dd. Read the comment docs.

@loosebazooka
Copy link
Member

There's some docs and code in examples that use auto:{}
for jib specifically: (/integration)/examples/jib-syc

@briandealwis
Copy link
Member Author

@loosebazooka 5f29e27 fixed the auto:{} in integration/examples/jib-sync, so they'll come through on the next release.

@loosebazooka
Copy link
Member

oh weird, I must have clicked on the commit or something

@loosebazooka
Copy link
Member

Looks fine to me, what happens when we need to configure auto in the future? should buildpacks not automatically enable sync?

@briandealwis
Copy link
Member Author

Auto sync is now enabled by default for buildpacks (#4656). Disabling requiring specifying a manual sync rule. Oops, I didn't update the docs!

@loosebazooka
Copy link
Member

How does that interact with the "one of" parsing of yaml? I'm not sure I remember what happens if the user injects some manual sync into the buildpacks artifact configuration?

@briandealwis
Copy link
Member Author

The oneOf is only checked at parse time: it has no effect on the underlying Go structs.

The auto-sync enablement happens as port-processing step, and only happens when there are no manual or inferred rules.

@briandealwis briandealwis added the docs-modifications runs the docs preview service on the given PR label Oct 19, 2020
@container-tools-bot
Copy link

Please visit http://35.235.68.162:1313 to view changes to the docs.

@container-tools-bot container-tools-bot removed the docs-modifications runs the docs preview service on the given PR label Oct 19, 2020
@gsquared94 gsquared94 merged commit b4c29e0 into GoogleContainerTools:master Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto-sync should be able to be disabled explicitly
4 participants