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

presence: introduce new module parameter force_delete #601

Merged
merged 3 commits into from May 7, 2016

Conversation

eschmidbauer
Copy link
Contributor

  • deletes expired presentity records without sending a NOTIFY
  • force_delete disabled by default

 - deletes expired presentity records without sending a NOTIFY
 - `force_delete` disabled by default
@eschmidbauer
Copy link
Contributor Author

@miconda here is PR for new presence parameter

@phil-lavin
Copy link
Contributor

phil-lavin commented May 6, 2016

Good timing. Literally just came out of a meeting where the undesirable behavior of repeated terminated NOTIFY messages was discussed. This is a useful addition.

You've changed a lot of whitespace that is not related to this PR. Makes it very difficult to digest :/

Appreciate this isn't your doing and you've just copied the LM_ERR from the rest of the code, but the log levels throughout look a little enthusiastic. These are debug logs - perhaps info at best.

@eschmidbauer
Copy link
Contributor Author

@phil-lavin sorry about the whitespace changes, I learned from my mistake and will fix whitespace in a separate commit from now on. The only significant change to the code is here:
if (delete_presentity(&pres) < 0) { LM_ERR("Deleting presentity\n"); goto error; }

And if you look at full code, you'll see I just copied it from a few lines below it. You are welcome to change the log levels in a separate PR!
Thanks,
Emmanuel

@miconda
Copy link
Member

miconda commented May 7, 2016

Thanks, it will be merged!

@miconda miconda merged commit 728c009 into master May 7, 2016
@linuxmaniac linuxmaniac deleted the feature/presence-force_delete_presentity branch May 9, 2016 08:10
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.

None yet

3 participants