Skip to content
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

Add temporal_route end-date #6664

Open
wants to merge 6 commits into
base: 4.3
Choose a base branch
from
Open

Add temporal_route end-date #6664

wants to merge 6 commits into from

Conversation

supapo
Copy link

@supapo supapo commented Nov 8, 2020

I've recreated the PR since there was some issue with the git unstaged files

@supapo
Copy link
Author

supapo commented Nov 8, 2020

What is the issue now?

@supapo
Copy link
Author

supapo commented Nov 11, 2020

I can see this log, but what does it mean?

TEST KazooTests\Applications\Crossbar\SystemConfigConferencesSection (testDefaultApplied) ... ERROR 0.01s too_many_requests
TEST KazooTests\Applications\Crossbar\SystemConfigConferencesSection (testDefaultIsSubtracted) ... ERROR 0.00s too_many_requests
TEST KazooTests\Applications\Crossbar\SystemConfigConferencesSection ... OK 0.00s

Fatal error: Uncaught Kazoo\Api\Exception\RateLimit: too_many_requests in /home/user/make-busy/vendor/2600hz/kazoo-php-sdk/lib/hz2600/Kazoo/HttpClient/HttpClient.php:94
Stack trace:
#0 /home/user/make-busy/vendor/guzzlehttp/promises/src/FulfilledPromise.php(39): Kazoo\HttpClient\HttpClient->Kazoo\HttpClient{closure}(Object(GuzzleHttp\Psr7\Response))
#1 /home/user/make-busy/vendor/guzzlehttp/promises/src/TaskQueue.php(47): GuzzleHttp\Promise\FulfilledPromise::GuzzleHttp\Promise{closure}()
#2 /home/user/make-busy/vendor/guzzlehttp/promises/src/Promise.php(246): GuzzleHttp\Promise\TaskQueue->run(true)
#3 /home/user/make-busy/vendor/guzzlehttp/promises/src/Promise.php(223): GuzzleHttp\Promise\Promise->invokeWaitFn()
#4 /home/user/make-busy/vendor/guzzlehttp/promises/src/Promise.php(267): GuzzleHttp\Promise\Promise->waitIfPending()
#5 /home/user/make-busy/vendor/guzzlehttp/promises/src/Promise.php(225): GuzzleHttp\Promise\Promise->invokeWaitList()
#6 /home/user/make-busy/vendor/guzzlehttp/promises/src/Promise.php(62): Guzzle in /home/user/make-busy/vendor/2600hz/kazoo-php-sdk/lib/hz2600/Kazoo/HttpClient/HttpClient.php on line 94

@supapo
Copy link
Author

supapo commented May 18, 2021

When this PR will be merged?

@@ -29,6 +29,12 @@
"description": "Whether the rule is enabled",
"type": "boolean"
},
"end_date": {
"default": 0,
Copy link
Member

Choose a reason for hiding this comment

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

Generally, we prefer that the absence of the value would mean no end date. Remove the default; end_date should only have a value if it is a proper, meaningful timestamp.

"end_date": {
"default": 0,
"description": "The date that the rule ends. Zero means no end date",
"support_level": "supported",
Copy link
Member

Choose a reason for hiding this comment

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

support_level is a 2600Hz-only field to include, please remove.

@@ -51,6 +52,7 @@
,ordinal = ?RULE_DEFAULT_ORDINAL :: ordinal()
,month = ?RULE_DEFAULT_MONTH :: kz_time:month()
,start_date = ?RULE_DEFAULT_START_DATE :: kz_time:date()
,end_date = ?RULE_DEFAULT_END_DATE :: kz_time:date()
Copy link
Member

Choose a reason for hiding this comment

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

Should default to 'undefined' instead of the tuple. Type would be kz_time:date() | 'undefined'

-spec check_end_date(non_neg_integer(), kz_term:ne_binary(), kz_time:datetime(), integer(), kzd_temporal_rules:doc()) -> 'future' | 'equal' | 'past'.
check_end_date(LSec, TZ, Now, EndDateSeconds, RulesDoc) ->
% TODO, this is an workaround since the from_gregorian_seconds function are crashing on low values such as zero
case EndDateSeconds of
Copy link
Member

Choose a reason for hiding this comment

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

Check the value for 'undefined' instead of 0, and I would do it in a function clause:

check_end_date(_LSec, _TZ, _Now, 'undefined', _RulesDoc) -> 'future';
check_end_date(...) ->

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants