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

[doc] Improve documentation for concurrency settings. #3491

Merged
merged 1 commit into from
Jan 16, 2020

Conversation

dgageot
Copy link
Contributor

@dgageot dgageot commented Jan 10, 2020

@dgageot dgageot added docs-modifications runs the docs preview service on the given PR and removed cla: yes size/M labels Jan 10, 2020
@container-tools-bot container-tools-bot removed the docs-modifications runs the docs preview service on the given PR label Jan 10, 2020
@container-tools-bot
Copy link

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

@codecov
Copy link

codecov bot commented Jan 10, 2020

Codecov Report

Merging #3491 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted Files Coverage Δ
pkg/skaffold/schema/latest/config.go 100% <ø> (ø) ⬆️
...affold/kubernetes/portforward/kubectl_forwarder.go 68.29% <0%> (+2.43%) ⬆️

@dgageot dgageot changed the title Improve documentation for concurrency settings. [doc] Improve documentation for concurrency settings. Jan 13, 2020
docs/content/en/docs/pipeline-stages/builders/_index.md Outdated Show resolved Hide resolved
docs/content/en/docs/pipeline-stages/builders/_index.md Outdated Show resolved Hide resolved
docs/content/en/docs/pipeline-stages/builders/_index.md Outdated Show resolved Hide resolved

Skaffold can also build multiple artifacts in parallel, by settings a value higher than `1` to `concurrency`.
For local builds, this is however disabled by default since local builds could have side effects that are
Copy link
Member

Choose a reason for hiding this comment

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

Do you think we should mention specific builders that are believed to be safe (docker, Bazel) or that aren't safe (and the circumstances)?

docs/content/en/docs/pipeline-stages/builders/_index.md Outdated Show resolved Hide resolved
**Faster builds**

If you are deploying to [local cluster]({{<relref "/docs/environment/local-cluster" >}}),
Skaffold will default `push` to `false` to speed up builds.
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should expand on this paragraph because there are a few implications.

  • certain clusters, like minikube, kind, Docker Kubernetes, are considered as local clusters
  • Skaffold can usually load the images directly into a local cluster, instead of via an image registry
  • local clusters change the default for push to false to avoid pushing the image to the remote repository
  • this means that the same builds can fail when switching the kube context to a remote cluster. but the speed tradeoff is worth it.

And maybe Faster builds isn't the right section heading either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to add some more documentation when most of this is merged :-)

@googlebot

This comment has been minimized.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Signed-off-by: David Gageot <david@gageot.net>
@nkubala nkubala merged commit 9fbacf4 into GoogleContainerTools:master Jan 16, 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.

5 participants