[sssd PR#690][opened] DYNDNS: Convert dyndns timer to be_ptask

[thalman]

URL: https://github.com/SSSD/sssd/pull/690
Author: thalman
 Title: #690: DYNDNS: Convert dyndns timer to be_ptask
Action: opened

PR body:
"""
We have API for periodic tasks and we want to
use it instead of tevent timers.

Resolves:
https://pagure.io/SSSD/sssd/issue/1944
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/690/head:pr690
git checkout pr690

[sssd PR#690][comment] DYNDNS: Convert dyndns timer to be_ptask

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/690
Title: #690: DYNDNS: Convert dyndns timer to be_ptask

pbrezina commented:
"""
Where did you get this 60 seconds limit? I can not find it in original code:
```c
ad_dyndns_init:
if (period < 60) period = 60;
```
Also please remove `be_nsupdate_init_timer` as it is now only used in tests and update tests. Otherwise it looks good.
"""

See the full comment at https://github.com/SSSD/sssd/pull/690#issuecomment-436246385

[sssd PR#690][comment] DYNDNS: Convert dyndns timer to be_ptask

[thalman]

URL: https://github.com/SSSD/sssd/pull/690
Title: #690: DYNDNS: Convert dyndns timer to be_ptask

thalman commented:
"""
This limit was not in code, only in documentation (sssd-ad.5.xml).

It is also inconsistent with sssd-ipa where there is no such limit and also default value for dyndns_refresh_interval is 0 in ipa (86400 for AD).
"""

See the full comment at https://github.com/SSSD/sssd/pull/690#issuecomment-436253672

[sssd PR#690][synchronized] DYNDNS: Convert dyndns timer to be_ptask

[thalman]

URL: https://github.com/SSSD/sssd/pull/690
Author: thalman
 Title: #690: DYNDNS: Convert dyndns timer to be_ptask
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/690/head:pr690
git checkout pr690

[sssd PR#690][comment] DYNDNS: Convert dyndns timer to be_ptask

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/690
Title: #690: DYNDNS: Convert dyndns timer to be_ptask

pbrezina commented:
"""
Please remove `be_nsupdate_init_timer` and the whole request `sdap_dyndns_timer_conn_send` from the code since it is not used anymore. However, you need to implement the functionality from `sdap_dyndns_timer_conn_send` in `ad_dyndns_update_send` (same for ipa).

`sdap_dyndns_timer_conn_send` calls `sdap_id_op_send` which basically connects to LDAP and populates service attributes (e.g. `ctx->service->sdap->uri`). Since we do not need the other logic that is in `sdap_dyndns_timer_conn_send` you should call `sdap_id_op_send` directly in `ad_dyndns_update_send` and `ipa_dyndns_update_send`.
"""

See the full comment at https://github.com/SSSD/sssd/pull/690#issuecomment-439344093

[sssd PR#690][+Changes requested] DYNDNS: Convert dyndns timer to be_ptask

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/690
Title: #690: DYNDNS: Convert dyndns timer to be_ptask

Label: +Changes requested

[sssd PR#690][synchronized] DYNDNS: Convert dyndns timer to be_ptask

[thalman]

URL: https://github.com/SSSD/sssd/pull/690
Author: thalman
 Title: #690: DYNDNS: Convert dyndns timer to be_ptask
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/690/head:pr690
git checkout pr690

[sssd PR#690][-Changes requested] DYNDNS: Convert dyndns timer to be_ptask

[thalman]

URL: https://github.com/SSSD/sssd/pull/690
Title: #690: DYNDNS: Convert dyndns timer to be_ptask

Label: -Changes requested

[sssd PR#690][comment] DYNDNS: Convert dyndns timer to be_ptask

[thalman]

URL: https://github.com/SSSD/sssd/pull/690
Title: #690: DYNDNS: Convert dyndns timer to be_ptask

thalman commented:
"""
> Where did you get this 60 seconds limit? I can not find it in original code:
> ad_dyndns_init:
> if (period < 60) period = 60;
>
> Also please remove `be_nsupdate_init_timer` as it is now only used in tests and update tests.
> Otherwise it looks good.

I removed this part, I found that it is implemented elsewhere different way.

"""

See the full comment at https://github.com/SSSD/sssd/pull/690#issuecomment-442846007

[sssd PR#690][comment] DYNDNS: Convert dyndns timer to be_ptask

[thalman]

URL: https://github.com/SSSD/sssd/pull/690
Title: #690: DYNDNS: Convert dyndns timer to be_ptask

thalman commented:
"""
> Where did you get this 60 seconds limit? I can not find it in original code:
> ad_dyndns_init:
> if (period < 60) period = 60;
>
> Also please remove `be_nsupdate_init_timer` as it is now only used in tests and update tests.
> Otherwise it looks good.

I removed this part, I found that it is implemented elsewhere different way.

"""

See the full comment at https://github.com/SSSD/sssd/pull/690#issuecomment-442846007

[sssd PR#690][comment] DYNDNS: Convert dyndns timer to be_ptask

[thalman]

URL: https://github.com/SSSD/sssd/pull/690
Title: #690: DYNDNS: Convert dyndns timer to be_ptask

thalman commented:
"""
> Where did you get this 60 seconds limit? I can not find it in original code:
> ad_dyndns_init:
> if (period < 60) period = 60;

I removed this part, I found that it is implemented elsewhere different way.

"""

See the full comment at https://github.com/SSSD/sssd/pull/690#issuecomment-442846007

[sssd PR#690][comment] DYNDNS: Convert dyndns timer to be_ptask

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/690
Title: #690: DYNDNS: Convert dyndns timer to be_ptask

pbrezina commented:
"""
Where is it implemented?

Ack to the patches!
"""

See the full comment at https://github.com/SSSD/sssd/pull/690#issuecomment-444442609

[sssd PR#690][+Accepted] DYNDNS: Convert dyndns timer to be_ptask

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/690
Title: #690: DYNDNS: Convert dyndns timer to be_ptask

Label: +Accepted

[sssd PR#690][comment] DYNDNS: Convert dyndns timer to be_ptask

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/690
Title: #690: DYNDNS: Convert dyndns timer to be_ptask

jhrozek commented:
"""
* master: df9e4802c060fc21d38f238265805092352e5c95
"""

See the full comment at https://github.com/SSSD/sssd/pull/690#issuecomment-444620631

[sssd PR#690][closed] DYNDNS: Convert dyndns timer to be_ptask

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/690
Author: thalman
 Title: #690: DYNDNS: Convert dyndns timer to be_ptask
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/690/head:pr690
git checkout pr690

[sssd PR#690][+Pushed] DYNDNS: Convert dyndns timer to be_ptask

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/690
Title: #690: DYNDNS: Convert dyndns timer to be_ptask

Label: +Pushed