feat: Support Bucket/Object lock operations #374
Conversation
googlebot
added
the
cla: yes
ghost
assigned 
stephenplusplus
added
the
do not merge
|
Just a ping @frankyn that this needs an official approval again (nothing has changed since the last PR I accidentally merged). That way, we're good to go when release time comes. |
There was a problem hiding this comment.
Thanks @stephenplusplus, I think one additional convenience method similar to getExpirationDate() would be helpful for RetentionPolicy.effectiveTime.
|
Sorry @stephenplusplus, it'd be better to just return the entire retentionPolicy object instead of just the |
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
googlebot
added
cla: no
|
@frankyn @stephenplusplus is this something we can update and merge? |
frankyn
added
cla: yes
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) |
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
googlebot
added
cla: no
frankyn
force-pushed
the
revert-373-revert-320-spp--bucket-lock
branch
from
1fc4b79 to
6f85575
Compare
stephenplusplus
force-pushed
the
revert-373-revert-320-spp--bucket-lock
branch
from
6f85575 to
161dd95
Compare
|
@JustinBeckwith ! We're trying to fit this change in with the TypeScript changes that have come recently. I'm getting lots of errors trying to run $ npm t
> @google-cloud/[email protected] test /Users/stephen/dev/nodejs-storage
> npm run test-only
> @google-cloud/[email protected] pretest-only /Users/stephen/dev/nodejs-storage
> npm run compile
> @google-cloud/[email protected] compile /Users/stephen/dev/nodejs-storage
> tsc -p .
system-test/storage.ts:905:14 - error TS2339: Property 'then' does not exist on type 'void'.
905 .then(() => bucket.getMetadata())
~~~~
system-test/storage.ts:919:14 - error TS2339: Property 'then' does not exist on type 'void'.
919 .then(() => bucket.setRetentionPeriod(RETENTION_DURATION_SECONDS))
~~~~
system-test/storage.ts:934:14 - error TS2339: Property 'then' does not exist on type 'void'.
934 .then(() => bucket.setRetentionPeriod(RETENTION_DURATION_SECONDS))
~~~~
system-test/storage.ts:949:14 - error TS2339: Property 'then' does not exist on type 'void'.
949 .then(() => bucket.setRetentionPeriod(RETENTION_DURATION_SECONDS))
~~~~
system-test/storage.ts:971:14 - error TS2339: Property 'then' does not exist on type 'void'.
971 .then(() => FILE.save('data'));
~~~~
system-test/storage.ts:984:14 - error TS2339: Property 'then' does not exist on type 'void'.
984 .then(response => {
~~~~
system-test/storage.ts:1000:14 - error TS2339: Property 'then' does not exist on type 'void'.
1000 .then(response => {
~~~~
system-test/storage.ts:1015:41 - error TS2339: Property 'then' does not exist on type 'void'.
1015 return FILE.getExpirationDate().then(response => {
~~~~
system-test/storage.ts:1030:20 - error TS2345: Argument of type 'File' is not assignable to parameter of type 'never'.
1030 FILES.push(file);
~~~~
system-test/storage.ts:1044:16 - error TS2339: Property 'setMetadata' does not exist on type 'never'.
1044 file.setMetadata({temporaryHold: null}, err => {
~~~~~~~~~~~
system-test/storage.ts:1050:18 - error TS2339: Property 'delete' does not exist on type 'never'.
1050 file.delete(next);Any words of wisdom as I stare down these angry lines? |
frankyn
added
cla: yes
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) |
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
googlebot
removed
the
cla: yes
googlebot
added
the
cla: no
|
I went with just using the callback style for the methods that had issues, and it seems to work now. System tests work locally for me. |
|
From System tests |
|
@JustinBeckwith could the system test errors above just be a Kokoro thing? Can anyone verify they're failing locally for them as well? |
|
@stephenplusplus when I run this locally I see the following: |
frankyn
added
the
kokoro:force-run
kokoro-team
removed
the
kokoro:force-run
frankyn
force-pushed
the
revert-373-revert-320-spp--bucket-lock
branch
from
995864a to
b3e501a
Compare
JustinBeckwith
added
cla: yes
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) |
JustinBeckwith
changed the title
|
@kinwa91 are we close on the system tests for kokoro passing here on master? If not, can you manually verify this PR is gtg? |
JustinBeckwith
force-pushed
the
revert-373-revert-320-spp--bucket-lock
branch
from
ae64919 to
c39d2e8
Compare
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
googlebot
added
cla: no
JustinBeckwith
force-pushed
the
revert-373-revert-320-spp--bucket-lock
branch
from
c39d2e8 to
117553f
Compare
|
CLAs look good, thanks! |
googlebot
added
cla: yes
|
Well @frankyn I'm pretty sure we fixed the system tests and exhausted the GCS bucket create quota 🤣 If you can fix that on long-door-651 this should be gtg. Anything I can do to help: |
JustinBeckwith
added
the
kokoro:force-run
kokoro-team
removed
the
kokoro:force-run
JustinBeckwith
added
the
kokoro:force-run
kokoro-team
removed
the
kokoro:force-run
This is a recreation of the original PR: #320. Head there for discussions held during development of this feature.
To Dos
References
This introduces new behavior:
Buckets
bucket#lock()
Lock a previously-defined retention policy. This will prevent changes to the policy.
bucket#removeRetentionPeriod()
Remove an already-existing retention policy from this bucket.
bucket#setRetentionPeriod(durationInSeconds)
Lock all objects contained in the bucket, based on their creation time.
Files
Get a Date object representing the earliest time this file will expire.
Additionally,
bucket#getMetadata(),bucket#setMetadata(),file#getMetadata(), andfile#setMetadata()are available if users wish to interact with the raw resource schemas as defined by the API.