-
Notifications
You must be signed in to change notification settings - Fork 641
Add bucket#makePublic and bucket#makePrivate #418
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
Conversation
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
I know the worst thing someone reviewing your PR can do is be like "here's a whole other bunch of code that renders yours useless!" I didn't intend to let it get to that point- I just wanted to drop the PR in my editor, which makes it easier for me to make sense of. I found some things I thought I could improve by demonstrating as opposed to trying to explain. And, before you know it... https://gist.github.com/stephenplusplus/66bfc7ba04ba97a8a15a I think either approach is perfectly fine, I just wanted to hold less state outside of each function, and make it more modular in general. As an example, I let It's not tested (didn't even run it), but if it's something that doesn't look hideous to you, I can send a PR with it to evolve from this one, or you can send me a meme that explains what a jerk I am, and I'll delete the gist and pretend it never happened! |
|
Hey this is great! I don't mind one bit because mine was more POC. One thing that is still TODO that needs to be thought of is the |
|
Upon closer inspection your async.eachLimit doesn't really make sense. You don't want to run with the nextQuery on each file, only after all files are processed. |
|
Ooh, good catch. I'll have to think my way out of that one! |
|
Actually you're right, I was messing up the callback count. |
|
Wait: async.eachLimit(files, MAX_PARALLEL_LIMIT, processFile, function(err) {Won't that process files (max 10 at a time) until all files are processed, then call processFiles again for the nextQuery? |
|
We're a mess |
|
Yeah my bad... :( I think you'll be good. Or you'll figure it out! 😄 |
|
Going to close this to let #429 carry it on. If I missed anything in the transfer, please update the |
…cloud-rad for nodejs (#418) This PR was generated using Autosynth. 🌈 Synth log will be available here: https://source.cloud.google.com/results/invocations/b02a7220-ded6-43ea-abe6-c043d792fee2/targets - [ ] To automatically regenerate this PR, check this box. Source-Link: googleapis/synthtool@21f1470 Source-Link: googleapis/synthtool@d82decc
This PR was generated using Autosynth. 🌈 Synth log will be available here: https://source.cloud.google.com/results/invocations/c503f640-90ae-4547-bcc8-a154d32c609e/targets - [ ] To automatically regenerate this PR, check this box. Source-Link: googleapis/synthtool@15013ef
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@google-cloud/projectify](https://togithub.com/googleapis/nodejs-projectify) | dependencies | major | [`^1.0.0` -> `^2.0.0`](https://renovatebot.com/diffs/npm/@google-cloud%2fprojectify/1.0.4/2.0.0) | --- ### Release Notes <details> <summary>googleapis/nodejs-projectify</summary> ### [`v2.0.0`](https://togithub.com/googleapis/nodejs-projectify/blob/master/CHANGELOG.md#​200-httpswwwgithubcomgoogleapisnodejs-projectifycomparev104v200-2020-03-24) [Compare Source](https://togithub.com/googleapis/nodejs-projectify/compare/v1.0.4...v2.0.0) ##### ⚠ BREAKING CHANGES - [email protected] introduced some breaking changes - drop Node 8 from engines field ([#​172](https://togithub.com/googleapis/nodejs-projectify/issues/172)) ##### Features - drop Node 8 from engines field ([#​172](https://www.github.com/googleapis/nodejs-projectify/issues/172)) ([3eac424](https://www.github.com/googleapis/nodejs-projectify/commit/3eac424bfb1ee47144a77888dc68db687988945e)) ##### Build System - update to latest version of gts/typescript ([#​171](https://www.github.com/googleapis/nodejs-projectify/issues/171)) ([30f90cc](https://www.github.com/googleapis/nodejs-projectify/commit/30f90cc172da6ed9394da91869556bf5eef42434)) ##### [1.0.4](https://www.github.com/googleapis/nodejs-projectify/compare/v1.0.3...v1.0.4) (2019-12-05) ##### Bug Fixes - **publish:** publication failed to reach npm ([#​141](https://www.github.com/googleapis/nodejs-projectify/issues/141)) ([5406ba5](https://www.github.com/googleapis/nodejs-projectify/commit/5406ba5e1d43a228a19072023c1baebce34190af)) ##### [1.0.3](https://www.github.com/googleapis/nodejs-projectify/compare/v1.0.2...v1.0.3) (2019-12-05) ##### Bug Fixes - **deps:** pin TypeScript below 3.7.0 ([6c95307](https://www.github.com/googleapis/nodejs-projectify/commit/6c953070139a77d30c4ce5b7dee1443874046906)) ##### [1.0.2](https://www.github.com/googleapis/nodejs-projectify/compare/v1.0.1...v1.0.2) (2019-11-14) ##### Bug Fixes - **docs:** add jsdoc-region-tag plugin ([#​135](https://www.github.com/googleapis/nodejs-projectify/issues/135)) ([59301e7](https://www.github.com/googleapis/nodejs-projectify/commit/59301e7cfa855add4894dd9c46870e61fffa7413)) ##### [1.0.1](https://www.github.com/googleapis/nodejs-projectify/compare/v1.0.0...v1.0.1) (2019-06-26) ##### Bug Fixes - **docs:** link to reference docs section on googleapis.dev ([#​119](https://www.github.com/googleapis/nodejs-projectify/issues/119)) ([90a009f](https://www.github.com/googleapis/nodejs-projectify/commit/90a009f)) </details> --- ### Renovate configuration :date: **Schedule**: "after 9am and before 3pm" (UTC). :vertical_traffic_light: **Automerge**: Disabled by config. Please merge this manually once you are satisfied. :recycle: **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. :no_bell: **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#googleapis/nodejs-compute).
🤖 I have created a release \*beep\* \*boop\* --- ## [2.0.0](https://www.github.com/googleapis/nodejs-compute/compare/v1.2.0...v2.0.0) (2020-05-08) ### ⚠ BREAKING CHANGES * update to latest version of gts and typescript (#433) * require node 10 in engines field (#431) ### Features * require node 10 in engines field ([#431](https://www.github.com/googleapis/nodejs-compute/issues/431)) ([fe2897f](https://www.github.com/googleapis/nodejs-compute/commit/fe2897fd1d625df0ccf5b910ec850e761c2ed6d3)) ### Bug Fixes * apache license URL ([#468](https://www.github.com/googleapis/nodejs-compute/issues/468)) ([#427](https://www.github.com/googleapis/nodejs-compute/issues/427)) ([02d63ed](https://www.github.com/googleapis/nodejs-compute/commit/02d63ed5a5a3559be0b5e49ed4c771b3661518a1)) * **deps:** update dependency @google-cloud/common to v3 ([#421](https://www.github.com/googleapis/nodejs-compute/issues/421)) ([d4469b6](https://www.github.com/googleapis/nodejs-compute/commit/d4469b693745df5a8a36b6988b7dba98f7d55142)) * **deps:** update dependency @google-cloud/paginator to v3 ([#419](https://www.github.com/googleapis/nodejs-compute/issues/419)) ([e667aee](https://www.github.com/googleapis/nodejs-compute/commit/e667aeecf7b9f51875c06cb6a885040828dc1181)) * **deps:** update dependency @google-cloud/projectify to v2 ([#418](https://www.github.com/googleapis/nodejs-compute/issues/418)) ([b861540](https://www.github.com/googleapis/nodejs-compute/commit/b861540aa17118a7508c7c970d84864a9028f588)) * **deps:** update dependency @google-cloud/promisify to v2 ([#417](https://www.github.com/googleapis/nodejs-compute/issues/417)) ([eedce75](https://www.github.com/googleapis/nodejs-compute/commit/eedce75e0d3f10dfed8151f12150d38d032f6a27)) * **deps:** update dependency @sendgrid/mail to v7 ([#424](https://www.github.com/googleapis/nodejs-compute/issues/424)) ([17af0f4](https://www.github.com/googleapis/nodejs-compute/commit/17af0f425354b261f9afc8ab30e16e66b7193fec)) ### Build System * update to latest version of gts and typescript ([#433](https://www.github.com/googleapis/nodejs-compute/issues/433)) ([a4fa8cc](https://www.github.com/googleapis/nodejs-compute/commit/a4fa8cc21df7a894486832b05449fa7afc02b7ee)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
🤖 I have created a release \*beep\* \*boop\* --- ## [5.5.0](https://www.github.com/googleapis/nodejs-security-center/compare/v5.4.1...v5.5.0) (2021-08-23) ### Features * turns on self-signed JWT feature flag ([#417](https://www.github.com/googleapis/nodejs-security-center/issues/417)) ([ec4b179](https://www.github.com/googleapis/nodejs-security-center/commit/ec4b17916d42f9ff006b9c9d0c1a6c2dd0743fa7)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
* updated CHANGELOG.md [ci skip] * updated package.json [ci skip] * updated samples/package.json [ci skip]
* updated CHANGELOG.md [ci skip] * updated package.json [ci skip] * updated samples/package.json [ci skip]
This PR was generated using Autosynth. 🌈 Synth log will be available here: https://source.cloud.google.com/results/invocations/4de22315-84b1-493d-8da2-dfa7688128f5/targets - [ ] To automatically regenerate this PR, check this box. Source-Link: googleapis/synthtool@94421c4
Do not merge! No tests yet!
This is the implementation to make a bucket public and private. In my preliminary manually testing, it works great!
Optionally a
recursiveoption can be set totrueto set all the files in the bucket to public as well. QUESTION: Should this be default true? Also, I'm returning either a single error after the first error occurs if a file cannot be made public for some reason, or if they setforcetotruethen it will add those errors to an array and return that array of errors at the end.Additionally, I'm returning a list of the file objects that were successfully publicized/privatized.
I'm looking for feedback on general approach and big issues you can see.
Issues: bucket#makePrivate with
strict: truealways throws an error forbidden before it finishes, because it somehow doesn't recognize my account using service account key json to authenticate as an owner. Any idea how I can become the root owner of the bucket?TODO: Add
options.autoto apply a predefinedObjectACL on the bucket to allow new files to be created with the same permissions.Depends on #390.