Redistricting the same records over and over?

jimcrist commented 3 years ago

Since I was getting the error "Exceeded Queries per minute Quota" when limit=100, I changed it to limit=50 but set it to run hourly instead of daily. Now it says: Finished execution of Electoral API - Districts Lookup with result: Success (a:1:{i:0;s:24:"50 addresses districted.";}) every hour. But it's a small database and I think everything is districted if it can be, so how does it pick which records to look at? Thanks...

jimcrist commented 3 years ago

I just checked the ConfigAndLog file and even tho debug and backtrace are turned off, it's producing maybe 50 of these messages every hour. Even tho the Job Log is showing Success. How come? Looks like it's redistricting the same 50 records every hour. Thanks for your help!

Aug 05 12:30:07  [debug] Contacting Google Civic API with url: https://www.googleapis.com/civicinfo/v2/representatives?address=1814%20S%2044th%20St%20WAUKESHA%20WI&key=xxxxxxxxxxxxxxxxxxxxxxxxxxxx
Array
(
    [0] => electoral
)

joemcl commented 3 years ago

Jim you should remove your API key from here and generate a new one ASAP

On Thu, Aug 5, 2021 at 11:02 AM jimcrist @.***> wrote:

I just checked the ConfigAndLog file and even tho debug and backtrace are turned off, it's producing maybe 50 of these messages every hour. Even tho the Job Log is showing Success. How come? Thanks for your help!

jimcrist commented 3 years ago

Oops...thanks...

jmcclelland commented 3 years ago

One of our groups just burned through 30,000 cicero credits re-districting the same 1500 addresses over and over.

I'm pretty sure the problem is with this where clause.

However, the fix is not obvious to me. I can see two approaches:

The query seems to expect every lookup to result in something being inserted into the electoral_status.electoral_status_error_code field - which has a one-to-one relationship with the address table. However, in practice, only failed lookups insert any data into this field. So, we could fix this problem by ensuring that every lookup puts something into that table (it might make sense to rename it electoral_status_code so it's clearer). This change would be consistent with the user interface - which sticks a big electoral status accordian on the summary page that is confusingly empty for all successfully looked up addresses. If we go this route, then that will at least be populated with something.
Alternatively, we can change this query to instead search for any record with electoral_districts.electoral_districts_level set to NULL. I think every successful lookup will put something in this field. This is a bit simpler over all, but ... CiviCRM API4 doesn't seem to support is_multiple custom fields, so I can't make a simple one line change. We'd have to re-write the query either using api v3 or via straight sql.

@MegaphoneJon - I can help prepare a patch but wondering if you have any guidance or suggestions? We can't really use the extension without this fix

MegaphoneJon commented 2 years ago

Hi Jamie,

Sorry to hear about the problem! But I'm not sure that what you've observed lines up with what I see.

Could you say a bit more about the addresses that were redistricted? Were they successful or failed lookups?
$addressQuery->addWhere('electoral_status.electoral_status_error_code', 'IS NULL'); should filter out an address when there's a value in that field - and you're correct that only failed lookups insert data here. I can only see this line being the issue if it's re-processing failed addresses, but that doesn't seem to be what you're suggesting.
are you saying API4 doesn't support custom fields with is_multiple, or custom field groups with is_multiple? I'm pretty sure it supports both but not sure which to test.

jmcclelland commented 2 years ago

Woops. You are right about apiv4 supporting is_multiple. I just wasted a lot of time writing legacy SQL that I have since thrown out. :(.

The 1500 addersses that were re-districted were successfully districted. I just posted a new PR to fix what seems to be the problem: https://github.com/MegaphoneJon/com.jlacey.electoral/pull/7. It's the query that pulls candidate addresses to redistrict. If you run the scheduled job without a limit, it works fine - since it pulls all the candidate addresses at once and iterates over them.

But, if you set a limit, then the first time the schedule job runs, it pulls the first $limit addresses in the database. The second time it runs, it pulls the first $limit addresses in the database again. If we don't restrict that query to addresses with no existing district data, then the same addresses are pulled every time the job runs.

jimcrist commented 2 years ago

I agree there's a problem. I have limit=50 and it always says Success (a:1:{i:0;s:24:"50 addresses districted.";}) even though it's a small database and I know it there's not that many that need to be districted. It does seem to randomly make progress districting new records that need doing. Jim Crist 608-213-3201 (talk/text)"How can our politicians represent us, when they'repaid millions to represent someone else?"

On Wednesday, September 15, 2021, 03:12:45 PM CDT, Jamie McClelland ***@***.***> wrote:

Woops. You are right about apiv4 supporting is_multiple. I just wasted a lot of time writing legacy SQL that I have since thrown out. :(.

The 1500 addersses that were re-districted were successfully districted. I just posted a new PR to fix what seems to be the problem: #7. It's the query that pulls candidate addresses to redistrict. If you run the scheduled job without a limit, it works fine - since it pulls all the candidate addresses at once and iterates over them.

But, if you set a limit, then the first time the schedule job runs, it pulls the first $limit addresses in the database. The second time it runs, it pulls the first $limit addresses in the database again. If we don't restrict that query to addresses with no existing district data, then the same addresses are pulled every time the job runs.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

jmcclelland commented 2 years ago

Thanks Jon for merging https://github.com/MegaphoneJon/com.jlacey.electoral/pull/7. That seems to have fixed this issue for me.

@jimcrist - yes, I also saw in my logs that 1500 contacts were districted. I think it's the same issue and now - if you pull in the latest version via github - I think it should be resolved.

jimcrist commented 2 years ago

Ugh...I unzipped the new version over the old version, ran the job and it failed?! It's still called V3.0.1, right? I uninstalled the extension and reinstalled it and got the same result. Then I deleted all the files in the com.jlacey.electoral-master directory and reinstalled the extension and got the same result. Please help...thanks!

jmcclelland commented 2 years ago

I think this could be another, separate issue... I don't think this extension cleans up after it is removed.

You may need to take all of these steps:

Run the following sql query to expose the custom groups created by this extension in the user interface.

UPDATE civicrm_custom_group SET is_reserved = 0 WHERE name IN ('electoral_districts', 'electoral_status', 'official_info');

Now, check your custom groups and you should see three electoral custom field groups. Enter each custom group and delete all the fields, then delete the custom groups. You should also check if you have the "Official" contact sub type and if so, delete it.
Disable the electoral extension
Uninstall the electoral extension
Install the electoral extension

I think that should do it.

jimcrist commented 2 years ago

I thought this was just a simple change to the programming logic? Did the data structure really change? Jon, do I really have to start from scratch and redistrict all my records?

MegaphoneJon commented 2 years ago

@jimcrist It seems like you don't have a custom field that you should. On your Electoral tab, you should have a field called "Level". See this screenshot in the README, "Level" is the first field listed.

Also, when posting error messages, please paste the text instead of a screenshot? It's easier for others searching, copy/pasting, and is accessible to folks with visual impairments who use screen readers.

@jmcclelland I addressed the issue with uninstallation not cleaning up after itself in v3.

For that reason, I'm a bit mystified as to why it wouldn't be working on uninstall/reinstall.

jimcrist commented 2 years ago

Yes, I have a Level field.

MegaphoneJon commented 2 years ago

@jimcrist I'm a bit mystified why it said "invalid field" then. Do you have access to the MySQL database? I could give you a couple of queries to run and you could post the results and that'll probably tell us why you're getting an error.

jimcrist commented 2 years ago

I followed @jmcclelland instructions...the SQL part went well, but when I tried to disable/uninstall the extension I got "DB Error: no such table". So, I restored the whole website/database. This time I did the uninstall first, which eliminated the electoral data. I uploaded the com.jlacey.electoral-master.zip file to the ext directory. Deleted the old extension. Unzipped the new one. Installed the Extension using the UI. Edited the Scheduled Job. Ran it and got the same damn error: Error message: Invalid field 'electoral_districts.electoral_level' @MegaphoneJon just saw your new comment. Yes, I can look at the SQL database.

jimcrist commented 2 years ago

The problem is obvious now, right? The installer is putting the word "districts" in a bunch of the database field names. I can't imagine how it worked for @jmcclelland. He must have just copied in the AbstractAPI.php file and never did an uninstall / install with the UI? The fact that the uninstall deletes all the data might be a real problem for someone with a really large database.

jimcrist commented 2 years ago

Hi, it's been 17 days...is anything going on with this? My database has no districting data now and I need it...thanks...

MegaphoneJon commented 2 years ago

@jimcrist Sorry, no. This isn't in active use for me right now, so fixing this isn't currently in my schedule. You can revert to the pre-patched version, or patch the problem line.

jimcrist commented 2 years ago

How do I "revert to the pre-patched version"? I've spent the last 24hrs on this. I do uninstall, delete the stuff from the database and reinstall (see process below), but I get "DB already exists" when I install it. I tried to delete auto_install.xml and reinstall it and get the same thing. Then I fooled Civi into installing it with This. Then I run the scheduled job and I'm back to: Invalid field 'electoral_districts.electoral_level' (see earlier posts). Then I went back to V2 and that works, but since it redistricts the same records over and over again it'll take forever to do the whole database. Please post a link to the version that worked on May 30th. (see Issue) I'm desperate...please help!

Uninstall / Reinstall Process: • Disable ext • Uninstall ext • Once the CiviCRM database is selected, selected the civicrm_custom_group table and change the “is_reserved” field from 1 to 0 for electoral_districts & electoral_status • Drop electoral fields in civicrm_custom_field and civicrm_custom_group • Drop tables: civicrm_value_electoral_districts & civicrm_value_electoral_status • Drop table: CIVICRM_VALUE_OFFICIAL_INFO • Remove old extension: rm -r com.jlacey.electoral-master • Upload zip file to …/ext • Unzip com.jlacey.electoral-master.zip • Install extension (hit refresh first)

jimcrist commented 2 years ago

I download the zip file. Any chance that's give me something different that GitHub Clone? I never figured out how to do that...

jimcrist commented 2 years ago

I turned on Debug and Backtrace. It doesn't seem to matter...I get the same message every time in the Log:

Nov 08 08:15:19 [warning] Deprecated join alias 'state_province' used in APIv4 get. Should be changed to 'state_province_id' Caller: Civi\Api4\Query\Api4SelectQuery::autoJoinFK Array ( [civi.tag] => deprecated )

Nov 08 08:15:19 [warning] Deprecated join alias 'contact' used in APIv4 get. Should be changed to 'contact_id' Caller: Civi\Api4\Query\Api4SelectQuery::autoJoinFK Array ( [civi.tag] => deprecated )

Does that help?

jmcclelland commented 2 years ago

@jimcrist I'm also not able to provide support to you on this issue.

I can point out - though - that the presence of the word "district" in the field names might be a red-herring. The field name (as used by APIv4) is "electoral_level" while the column name in the database is "electoral_district_level". The API never directly references the actual column name in the database, it only references the name as it appears in the civicrm_custom_field table.

Also, any reference in your log to a deprecation should not be the cause of a problem - just a notice of a problem that might happen in the future.

MegaphoneJon commented 2 years ago

@jimcrist You can click "History" on the main page of this repository, then click on the far right button (< >) to see the code at that point in time. From there you can download a zip file.

None of that will help with a "DB Already Exists" error, and the debug/backtrace message you're showing doesn't correlate with that error.

As we're fond of saying, open source software is free to use but not free to write. If this is worth something to you, please consider hiring someone to assist you.

jimcrist commented 2 years ago

@MegaphoneJon I've been looking for a button like History, but I can't find it?! Maybe it's not turned on...like Issues wasn't before? I'm past the DB Already Exists. I manually entered the Ext into the civicrm_extension table. Or don't you think it's properly installed? The database tables & fields all look good. I can run the Scheduled Job for State Districting, but I get Invalid field 'electoral_districts.electoral_level'. Could I hire you...since you're most familiar with it?

MegaphoneJon commented 2 years ago

Hi Jim - do you still have my email address? Work is very busy right now, but I can see what I can do.

joemcl commented 2 years ago

@jimcrist re the commit history, see https://github.com/MegaphoneJon/com.jlacey.electoral/commits/master

@MegaphoneJon may be using GitHub Desktop, which indeed shows a History tab, see https://docs.github.com/en/desktop/contributing-and-collaborating-using-github-desktop/making-changes-in-a-branch/viewing-the-branch-history

jimcrist commented 2 years ago

Hi Jon, I found the commit history and downloaded the May 16th version, which installs without a hitch which is nice. But it still fails with a slightly different error: Invalid field 'electoral_status.electoral_status_error_code'. I've set it up in a staging environment that you could play with. I'm a retired programmer, but don't have a PHP background and don't have a debug environment set up. I just found your email address from when you helped me in 2015. Hopefully, it's the same. I just sent you an email.

jimcrist commented 2 years ago

Maybe a new clue: I got a Cloudways tech to look at the server logs as I ran the scheduled job for state level districting. This is a WordPress site. CiviCRM 5.40.1. The following error got logged multiple times: Undefined index: name in /home/xxxxxx.cloudwaysapps.com/xxxxxx/public_html/wp-content/uploads/civicrm/templates_c/en_US/%%37/379/37944059%%JobLog.tpl.php on line 34 PHP message: PHP Notice: Undefined index: command in /home/xxxxxx.cloudwaysapps.com/xxxxxx/public_html/wp-content/uploads/civicrm/templates_c/en_US/%%37/379/37944059%%JobLog.tpl.php on line 34 PHP message: PHP Notice: Undefined index: data in /home/xxxxxx.cloudwaysapps.com/xxxxxx/public_html/wp-content/uploads/civicrm/templates_c/en_US/%%37/379/37944059%%JobLog.tpl.php on line 34 And then the process finally died with this: [Sat Nov 20 18:53:18.890556 2021] [proxy_fcgi:error] [pid 18571:tid 140370612487936] [client 107.142.189.75:23450] AH01071: Got error 'pps.com/xxxxxx/public_html/wp-content/uploads/civicrm/templates_c/en_US/%%37/379/37944059%%JobLog.tpl.php on line 34 PHP message: PHP Notice: Undefined index: name in /home/xxxxxx.cloudwaysapps.com/xxxxxx/public_html/wp-content/uploads/civicrm/templates_c/en_US/%%37/379/37944059%%JobLog.tpl.php on line 34 PHP message: PHP Notice: Undefined index: command in /home/xxxxxx.cloudwaysapps.com/xxxxxx/public_html/wp-content/uploads/civicrm/templates_c/en_US/%%37/379/37944059%%JobLog.tpl.php on line 34 PHP message: PHP Notice: Undefined index: data in /home/xxxxxx.cloudwaysapps.com/xxxxxx/public_html/wp-content/uploads/civicrm/templates_c/en_US/%%37/379/37944059%%JobLog.tpl.php on line 34', referer: https://wordpress-xxxxxx-1876584.cloudwaysapps.com/wp-admin/admin.php?page=CiviCRM&q=civicrm%2Fadmin%2Fjob&reset=1

MegaphoneJon commented 2 years ago

@jmcclelland There seems to be an error in the PR #7 you submitted in response to this, which was causing @jimcrist 's error. See the commit I made today. You're calling a contact custom field from an Address.get call without joining to another entity. Once I added the join to the district data, this error cleared up.

@jimcrist I'm chasing down some minor related issues and will update you soon.

jmcclelland commented 2 years ago

Thanks for the fix @MegaphoneJon and the report @jimcrist, and sorry for sending you on a wild goose chase of debugging.

I tested this change on our version of CiviCRM (5.39 lts) and the fix seems to work fine. Somehow, it also works fine without the fix (we had a group successfully code 30,000 records). I suspect the older version of CiviCRM allowed the implicit join?

MegaphoneJon / com.jlacey.electoral

Redistricting the same records over and over? #3