-
Notifications
You must be signed in to change notification settings - Fork 4
Document naive-datetime host time zone behaviour; use aware datetimes in examples #55
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -237,7 +237,7 @@ Now let's see how to combine custom functions for a real-world application - a b | |
| ```python | ||
| from cel import Context, evaluate | ||
| import re | ||
| from datetime import datetime, timedelta | ||
| from datetime import datetime, timedelta, timezone | ||
|
|
||
| def validate_password(password): | ||
| """Validate password strength.""" | ||
|
|
@@ -253,10 +253,8 @@ def days_until_expiry(expiry_date_str): | |
| """Calculate days until expiry.""" | ||
| try: | ||
| expiry = datetime.fromisoformat(expiry_date_str.replace('Z', '+00:00')) | ||
| now = datetime.now() | ||
| # Remove timezone info for comparison | ||
| expiry_naive = expiry.replace(tzinfo=None) | ||
| delta = expiry_naive - now | ||
| now = datetime.now(timezone.utc) | ||
| delta = expiry - now | ||
| return max(0, delta.days) | ||
| except: | ||
| return 0 | ||
|
|
@@ -432,7 +430,7 @@ These patterns provide the foundation for production-ready systems: | |
| **Complete PolicyContext Implementation** | ||
| ```python | ||
| from cel import Context, evaluate | ||
| from datetime import datetime | ||
| from datetime import datetime, timezone | ||
|
|
||
| class PolicyContext: | ||
| """Reusable context builder for policy evaluation.""" | ||
|
|
@@ -444,11 +442,12 @@ class PolicyContext: | |
| def _setup_common_functions(self): | ||
| """Set up commonly used functions.""" | ||
| def current_time(): | ||
| return datetime.now() | ||
| # Timezone-aware, so the CEL timestamp is the same instant on every host. | ||
| return datetime.now(timezone.utc) | ||
|
|
||
| def is_business_hours(): | ||
| # For testing purposes, always return True | ||
| # In production, use: datetime.now().hour to check 9 <= hour <= 17 | ||
| # In production, use: datetime.now(timezone.utc).hour to check 9 <= hour <= 17 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For organizations whose business hours are not defined in UTC, this production recommendation checks the wrong clock—for example, 09:00 in Auckland is 20:00 or 21:00 UTC on the preceding day. Since only the integer hour is used and no datetime crosses into CEL here, switching to UTC does not address the naive-datetime conversion issue; the example should use the organization's explicit Useful? React with 👍 / 👎. |
||
| return True | ||
|
|
||
| def contains_any(text, keywords): | ||
|
|
@@ -488,7 +487,7 @@ class PolicyContext: | |
| "method": method, | ||
| "path": path, | ||
| "ip": ip_address, | ||
| "time": datetime.now().isoformat() | ||
| "time": datetime.now(timezone.utc).isoformat() | ||
| }) | ||
| return self | ||
|
|
||
|
|
||
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.
When
expiry_date_stris a valid ISO-8601 value without an offset,datetime.fromisoformat()returns a naive datetime, so subtracting the newly awarenowraisesTypeError; the broad handler then silently returns0and treats even a future subscription as expired. Either require and validate an offset explicitly or attach the intended timezone before performing the subtraction.Useful? React with 👍 / 👎.