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

Logs reflect Failure when include from SQL is carried out during nosync_time_ranges #1429

Closed
qosobrin opened this issue Jun 22, 2022 · 7 comments · Fixed by #1431
Closed
Labels
bug ready A PR is waiting to be merged. Close to be solved
Milestone

Comments

@qosobrin
Copy link
Contributor

qosobrin commented Jun 22, 2022

Version

6.2.60

Installation method

Debian Bullseye Package

Expected behavior

We have plenty of lists with subscribers defined in a SQL external database; all these definitions contain nosync_time_ranges. In my understanding, inclusion of subscribers should not be even started during this periods and logs should not collect any entry related to this. But if the desing of sympa does not allow to prevent this inclusion from being started and logs to be collected, then logs should not reflect a Failure since this is not a failure in the process but an regular behaviour.

Actual behavior

Logging collects entries like this one:

Jun 22 07:05:41 hostname task_manager[1210995]: info Sympa::Request::Handler::include::_twist() Sympa::Request <action=include;[email protected];role=member>: Failure, 0 added, 720 held, 0 updated

Steps to reproduce

Define an include_sql_query with nosync_time_ranges.

include_sql_query
name students
db_name expxcm-dsn
db_type ODBC
db_passwd PASSWORD
nosync_time_ranges 06:30-07:45
sql_query SELECT email FROM table WHERE role = 1
db_user USERNAME
@ikedas
Copy link
Member

ikedas commented Jun 22, 2022

Hi @qosobrin ,

The section "Steps to reproduce" should contain enough necessary and sufficient information to make the reader actually reproducing it. For example, please show us the lines you actually added to the configuration file.

@qosobrin
Copy link
Contributor Author

Hi @qosobrin ,

The section "Steps to reproduce" should contain enough necessary and sufficient information to make the reader actually reproducing it. For example, please show us the lines you actually added to the configuration file.

OK, @ikedas. I've just corrected my mistake.

@ikedas ikedas added the bug label Jun 23, 2022
@ikedas ikedas added the ready A PR is waiting to be merged. Close to be solved label Aug 11, 2022
@ikedas ikedas added this to the 6.2.72 milestone Sep 24, 2022
@ikedas
Copy link
Member

ikedas commented Nov 15, 2022

@qosobrin , if possible, could you please apply this patch and check if the problem will be solved?

@qosobrin
Copy link
Contributor Author

@qosobrin , if possible, could you please apply this patch and check if the problem will be solved?

Sorry, @ikedas. I've been busy lately at work and had no time to check github. I will let you know as soon as I try your patch. Thank you very much. Best regards.

@ikedas
Copy link
Member

ikedas commented Nov 20, 2022

I see. I'll merge my patch for now. If you notice any problems, please report them again.

Thank you for reporting bug!

@qosobrin
Copy link
Contributor Author

Sorry for the delay, @ikedas I have just tested the patch and it worked as expected:

Nov 23 10:17:34 hostname task_manager[2795]: info Sympa::Request::Handler::include::_twist() : Skipped

Thank you very much for your work.
Best regards.

@ikedas
Copy link
Member

ikedas commented Nov 24, 2022

@qosobrin , thank you for confirming bug fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ready A PR is waiting to be merged. Close to be solved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants