On #733054: Watchdog logging of all searches is performance hit; need ability to turn it off we added a new config setting in search.settings called 'logging'.

This controls whether searches are logged or not, in Drupal 8.

In previous versions, there was no setting, and all searches were logged.

In Drupal 8, the default value is FALSE (no logging).

So... might want to do something about this in Migrate? (I was asked to file a follow-up issue for Migrate)

CommentFileSizeAuthor
#7 2305839-7.patch2.12 KBultimike
#5 2305869-5.patch1.32 KBsvendecabooter
#3 2305869-3.patch1.31 KBsvendecabooter

Comments

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
commented

So, I guess the question is:

  1. Do we do nothing, and just let sites migrated from D6 and D7 have their search logging setting set to FALSE.
  2. Since logging was always on in D6 and D7, perhaps we want to create a migration that sets it to TRUE when a D6 or D7 site is migrated to D8.

Thoughts?

Thanks,
-mike

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

It seems to me that if you're migrating a d6/7 site into 8 and searches were logged there, to get an accurate migration you'd want the value TRUE in 8 as well to make searches logged. I believe that is why I was asked to file this issue.

svendecabooter’s picture

svendecabooter
he/him
Primary language Dutch
Location Gent
commented
Status: Active » Needs review
StatusFileSize
new 2305869-3.patch1.31 KB

Option 2 indeed seems logical.
Attached a patch that sets this default value. Feedback welcome

Status: Needs review » Needs work

The last submitted patch, 3: 2305869-3.patch, failed testing.

svendecabooter’s picture

svendecabooter
he/him
Primary language Dutch
Location Gent
commented
Status: Needs work » Needs review
StatusFileSize
new 2305869-5.patch1.32 KB

Was mixing integers and booleans. New patch in attachment.

benjy’s picture

benjy commented
Status: Needs review » Needs work

Can we add a test for this as well?

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
commented
Status: Needs work » Needs review
StatusFileSize
new 2305839-7.patch2.12 KB

Patch attached.

Thanks,
-mike

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Looks good to me! But I'm not an expert in Migrate in 8 so... will leave to someone else to RTBC.

benjy’s picture

benjy commented
Status: Needs review » Reviewed & tested by the community

I agree that the default value as per D6/D7 should be migrated. Patch looks good. RTBC

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

This issue is a prioritized change as per #2350615: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase? and it's benefits outweigh any disruption. Committed 9872c3a and pushed to 8.0.x. Thanks!

  • alexpott committed 9872c3a on 8.0.x 
    Issue #2305869 by svendecabooter, ultimike | jhodgdon: Search migration...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.