-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
deploy: add a "deploy" command #5599
Conversation
Hi Bjorn, ping? I have a follow-on PR that adds support for ordering the uploads (similar to bep/s3deploy#37). I can add another one to support the "ignore subfolders" feature request (bep/s3deploy#33). I would also like to add CloudFront support in a separate PR. |
@vangent I'm in the Canary Island doing mostly nothing this week. It's called a vacation. I will get back to this next week. |
OK no worries! Just wanted to check in. Have a relaxing vacation! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added a set of comments of things that I think needs to be adjusted. We probably need to go a round 2) with this, but that will have to wait until I get home from sunny Spain.
jww.FEEDBACK.Println("Success!") | ||
|
||
// TODO: Add support for CloudFront invalidation similar to s3deploy, | ||
// and possibly similar functionality for other providers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this not a feature in gocloud
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not currently, no; CDN APIs are very low on the list of APIs we have demand for. That may change.
IIUC, CloudFront can cache content from any provider.
I'd like to add CloudFront specifically, but in a separate PR if that's OK. We can see if there's demand for other CDNs; if there's a lot, then we can consider adding an abstraction to Go Cloud for it.
deploy/deploy.go
Outdated
|
||
// route represents configuration to be applied to files whose paths match | ||
// a specified pattern. | ||
type route struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need a way to set the ACL. In s3deploy this is a boolean flag (public or private, default private).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not supported by Go Cloud yet (google/go-cloud#1108).
It's a bit tricky because this functionality (setting the ACL per blob rather than per bucket/container) is not available in Azure at all.
Is it OK to not include this for now? I would guess that most people would have their site in a single bucket/container and can set the ACL there, no?
@vangent thanks, your updates look great. I was going to suggest that we pull his in and hide the But then I started looking at the dependency changeset (i.e. the There are several issues with this which may be blocking:
I'm not saying we should not any new dependencies, but to me, it looks like the dependency handling in gocloud.dev/blob/* should be rethought (make features optional, maybe) so we don't need to "pull in the world" to upload a file. /cc @spf13 |
Hi @bep, Thank you for Cc'ing me.
Very cool!
I am happy to report that many of the new dependencies, including the indirect ones, especially the four that you mentioned, are already in Debian. I would say we may need to make about a handful new packages, and that we on the Debian Go Team are used to doing that. Also, other Debian Go Team members had a similar discussion with other software, and they had taught me to work smarter, as in we can put these new dependencies in the vendor/ directory as part of the Hugo source package. I actually did that between July and September 2018 for github.com/wellington/go-libsass and github.com/bep/go-tocss until they got accepted by the Debian FTP masters (My fault for missing some of the copyright information, contributing to that delay) so that we could have Hugo 0.43 to 0.47.1 in Debian without being blocked by new dependencies. (But then Debian's golang-1.11 hit a snag and that I couldn't solve myself...) So, yes, no need to worry about the Debian and Ubuntu side of things. I am more than happy to use the same tricks again as before to keep the Debian package up-to-date.
Cool, thanks! Cheers, |
@anthonyfok thanks for that input. It does address some of the concerns, but the |
Yes, that's a project I have access to and I understand your concerns. Some of this may be due to Go modules being a bit confusing about dependencies -- for example, I reverted my changeds to I think that I did
That is still a long list, but it's not too surprising at least to me given the new dependencies on Azure, AWS, and GCP. I don't think there's any way around those if |
@bep does my response seem reasonable? |
I assume that is test dependencies, which will not end up in the end binary -- but certainly in a binary (the test binary).
The response is reasonable. But I have to consider the conclusion. The thing is, this discussion started right about at the same time as Russ Cox, a coworker of yours, posted this article about this subject. I took that as a sign that I should stop and think really hard about this. Which I'm doing. I will get back to you early next week. |
Not necessarily. Currently our repo has a single main I think that go list -deps is a more accurate way of viewing actual dependencies, not the |
Quick note that this is not forgotten. I'm thinking about pulling it in and add a "feature flag" of some sort (build tag), so people can use it if they want and we can enable it by default once it is done-done. But I have to wrap my head around it, because once it is in it is hard to take back. |
Hi @bep, any update on your "feature flag" idea? |
No, and sorry about the delay. |
@vangent here's what we're going to do:
|
SGTM. After this PR, I think all that's needed is updates to the documentation, which I'm happy to try but would like your review on. I have a another follow-on to support "ordering" of uploads (I think |
Can you squash the commits and rebase against master?
Does this PR implement CDN cache invalidation? If not, that is, in my head, a must have. |
Squash/rebase done. No, it doesn't include CDN invalidation. |
There are 12 open TODOs in the PR, please review them and let me know which ones can be deleted, which ones need more investigation, and which ones should definitely be done in follow-on PRs before considered this feature complete (e.g, like the CDN invalidation). Thanks! |
I have a PR ready after this one to add CDN invalidate for CloudFront. It just invalidates |
I think the main thing that needs to happen before this can be submitted is that the change to |
@vangent yes, don't worry about the config.toml. That does not go "live" until we do a release and push it to the hugoDocs repo (and even then it would not hurt anyone). I have triggered a new build, seems to be some "build exeeding limit" timeout issues. Could this be a Go Modules issue? I.e. that cloning all the dependencies is taking too long? If so, we should probably look into the newly announced modules proxy server. I will merge once I understand the above. I see the TODOs, and I will create separate issues of some of those so we can add them later (e.g. the most important that I would need to use this (and when I need it, I'll assume others also would), quickly noting here:
|
I'm doing some build testing experimentation of this in |
OK. I reverted the changes to that file since they were just an example and included (for example) my bucket names. |
It's possible that downloading the additional modules pushed it over some timeout edge. I have seen the checks pass on this PR before though. Enabling the modules proxy sounds like a good idea regardless. |
SGTM. Please go ahead and assign the issues to me. I already have PRs ready for:
I will start working on the "subfolders of a bucket" TODO as well. Thanks bep! |
I merged this in #5925 |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
The "deploy" command uses Go Cloud's
blob
package to deploy to multiple Cloud providers. Currently GCS, S3, and Azure are supported.A new "deployments" section is added to the site config file; multiple deployment targets can be configured and selected using
--target
flag.Deployment consists of walking the
public/
site, walking the remote target, and computing a diff. New local files are marked to be uploaded, files at the target that no longer exist locally are marked to be deleted, and local files that have changed (either in size or MD5 hash) are marked to be uploaded.The command prompts for confirmation before applying changes. The prompt can be disabled via
--skipConfirmation
. Alternatively,--dryRun
will show proposed changes without making them.Similar to
s3deploy
, a new "routes" section in the site config file can control metadata for the upload, including headers likeCache-Control
andContent-Type
, as well as whether the content should be gzipped.