Skip to content
This repository was archived by the owner on Mar 31, 2026. It is now read-only.

feat: add support for 'Blob.custom_time' and lifecycle rules - #199

Merged
frankyn merged 22 commits into
googleapis:masterfrom
MaxxleLLC:storage_issue_196
Aug 26, 2020
Merged

feat: add support for 'Blob.custom_time' and lifecycle rules#199
frankyn merged 22 commits into
googleapis:masterfrom
MaxxleLLC:storage_issue_196

Conversation

@HemangChothani

Copy link
Copy Markdown
Contributor

Fixes#196

@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 1, 2020
@HemangChothaniHemangChothani changed the title Storage issue 196feat(storage): add support of custom time metadata and timestampJul 1, 2020

@jkwluijkwlui left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @HemangChothani! Please hold until product is ready to move forward.

Comment threadgoogle/cloud/storage/blob.py
@tseavertseaver added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 24, 2020
@tseaver

Copy link
Copy Markdown
Contributor

I have marked this PR as do not merge per @jkwlui .

tseaver
tseaver previously requested changes Jul 24, 2020
Comment threadgoogle/cloud/storage/blob.py Outdated
Comment threadtests/unit/test_blob.py
Comment threadgoogle/cloud/storage/blob.py Outdated
See https://cloud.google.com/storage/docs/json_api/v1/objects

:type value: :class:`datetime.datetime`
:param value: (Optional) Set the custom time of blob. Datetime object parsed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Document the semantics of None value (which should be to clear any existing property value).

@HemangChothaniHemangChothani added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 28, 2020
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 28, 2020
@busunkim96busunkim96 reopened this Jul 31, 2020
@tseavertseaver changed the title feat(storage): add support of custom time metadata and timestampfeat: add support for 'Blob.custom_time' and lifecyccle rules.Aug 4, 2020
@tseavertseaver changed the title feat: add support for 'Blob.custom_time' and lifecyccle rules.feat: add support for 'Blob.custom_time' and lifecyccle rulesAug 4, 2020
Comment threadtests/unit/test_blob.py

@frankynfrankyn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for your patience @HemangChothani,

Update: The Cloud Storage team has updated type of noncurrent_time_before and custom_time to be a Date format (YYYY-mm-dd) instead of DateTime.

Discovery document with updated documentation with respect to these fields:
https://www.googleapis.com/discovery/v1/apis/storage/v1/rest

@HemangChothani

Copy link
Copy Markdown
ContributorAuthor

Update: The Cloud Storage team has updated type of noncurrent_time_before and custom_time to be a Date format (YYYY-mm-dd) instead of DateTime.

@frankyn That change has been already done.

Comment threadgoogle/cloud/storage/blob.py Outdated
See https://cloud.google.com/storage/docs/json_api/v1/objects

:rtype: :class:`datetime.date` or ``NoneType``
:returns: Date object parsed from iso8601 valid date, or

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be dateTime

@frankynfrankyn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have two nits for documentation, otherwise LGTM.

Comment threadgoogle/cloud/storage/blob.py Outdated

See https://cloud.google.com/storage/docs/json_api/v1/objects

:type value: :class:`datetime.datetime` or ``NoneType``

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Once set it can't be unset and only changed to a custom datetime in the future. If the custom_time must be unset, you must either perform a rewrite operation or upload the data again.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd state that setter can only be datetime.datetime to not confuse folks.

Comment threadgoogle/cloud/storage/bucket.py Outdated

:type custom_time_before: :class:`datetime.date`
:param custom_time_before: (Optional) Apply rule action to items whose custom time is before this
date. This condition is relevant only for versioned objects.

@frankynfrankynAug 24, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please include similar text to noncurrent_time_before with RFC3339 and an example.

@frankynfrankyn removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Aug 24, 2020

@frankynfrankyn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 nit, thanks @HemangChothani

Comment threadgoogle/cloud/storage/blob.py Outdated

See https://cloud.google.com/storage/docs/json_api/v1/objects

:type value: :class:`datetime.datetime` or ``NoneType``

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd state that setter can only be datetime.datetime to not confuse folks.

@frankyn
frankyn requested a review from tseaverAugust 26, 2020 17:19
@frankynfrankyn changed the title feat: add support for 'Blob.custom_time' and lifecyccle rulesfeat: add support for 'Blob.custom_time' and lifecycle rulesAug 26, 2020
@frankyn
frankyn dismissed tseaver’s stale reviewAugust 26, 2020 18:00

tseaver's feedback was addressed and dismissing as the PR LGTM.

@frankyn
frankyn merged commit 180873d into googleapis:masterAug 26, 2020

@custom_time.setter
def custom_time(self, value):
"""Set the custom time for the object. Once set it can't be unset

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please follow PEP 257 for Multi-line docstrings, i.e., move the non-summary down below.

"""Set the custom time for the object. Once set it can't be unset
and only changed to a custom datetime in the future. If the
custom_time must be unset, you must either perform a rewrite operation
or upload the data again.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These semantics are not enforced by the method.

@property
def days_since_custom_time(self):
"""Conditon's 'days_since_custom_time' value."""
return self.get("daysSinceCustomTime")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Setter for this property?

"""Conditon's 'custom_time_before' value."""
before = self.get("customTimeBefore")
if before is not None:
return datetime_helpers.from_iso8601_date(before)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Setter for this property?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Not sure about the setter method for all this properties, because we don't have a method for other properties as well which defined before. Please let me know if needed, so need to create setter method for all the properties of LifecycleRuleConditions class.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks both, looks like setters weren't defined at this level. It will be filed as a feature request for now. It doesn't block customers from using the feature.

cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
…pis#199)
* feat(storage): add support of custom time metadata and timestamp
* feat(storage): change the return type of custom_time_before
* feat(storage): add setter method
* feat(storage): add test for None value
* feat(storage): changes in unittest
* feat(storage): change custom_time type to date
* feat: change custom_time to datetime
* feat: nit
Co-authored-by: Jonathan Lui <jonathanlui@google.com>
Co-authored-by: Tres Seaver <tseaver@palladion.com>
Co-authored-by: Frank Natividad <frankyn@users.noreply.github.com>
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
…pis#199)
* feat(storage): add support of custom time metadata and timestamp
* feat(storage): change the return type of custom_time_before
* feat(storage): add setter method
* feat(storage): add test for None value
* feat(storage): changes in unittest
* feat(storage): change custom_time type to date
* feat: change custom_time to datetime
* feat: nit
Co-authored-by: Jonathan Lui <jonathanlui@google.com>
Co-authored-by: Tres Seaver <tseaver@palladion.com>
Co-authored-by: Frank Natividad <frankyn@users.noreply.github.com>
@release-pleaserelease-pleaseBot mentioned this pull request Jan 12, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support customTime metadata and custom timestamp based OLM

7 participants

@HemangChothani@tseaver@frankyn@jkwlui@googlebot@busunkim96@yoshi-kokoro