I need to override the memcache logger.

So the first step is to inject it.

CommentFileSizeAuthor
#6 logger-2779147-6.patch6.17 KBmartin107
#6 interdiff-2779147-2-6.txt3.18 KBmartin107
#2 deferWarning-2779147-2.patch7.34 KBmartin107

Comments

martin107 created an issue. See original summary.

martin107’s picture

martin107 commented
Status: Active » Needs review
StatusFileSize
new deferWarning-2779147-2.patch7.34 KB

triggering testbot

damiankloip’s picture

damiankloip commented

I'm not sure about this, we currently don't rely on watchdog() anymore, and you have injected the service before hand anyway. So the defer seems pointless now?

martin107’s picture

martin107 commented
Assigned: martin107 » Unassigned
Issue summary: View changes

Thank you for the review....

My position is I am trying to evaluate the usefulness of this module My needs demand I pipe all logs to an independent logging service. - I am not an expert on D8 bootstrapping,

Do I interpret you concerns correctly ?

DrupalMemcacheInterface::get()

That is the point, in general, where our code may generate a log. Does directly injecting the logger service delay the creation of memcache services - breaking things?

If things are not broken then I would still like to simplify and in my next patch I want to just remove the concept of deferring?

Is that correct?

damiankloip’s picture

damiankloip commented

Yes. Correct. I think we can remove the defer logic and use the logger. Your patch already injects it into the backend. So an instance is already needed when the backend instance is created. So the original reason for registering a shutdown function in previous drupal versions seems less of a problem now in d8.

If we hit issues in the future we will have to remember think but I think that should be good.

Thank you!

martin107’s picture

martin107 commented
StatusFileSize
new interdiff-2779147-2-6.txt3.18 KB
new logger-2779147-6.patch6.17 KB

That is good news -- I've stripped out the defer logic

damiankloip’s picture

damiankloip commented

This looks pretty good at a glance (looks like one newline missing but I can fix that on commit). I will commit this later on!

  • damiankloip committed ed98d69 on 8.x-2.x 
    Issue #2779147 by martin107, damiankloip: Inject the logger
damiankloip’s picture

damiankloip commented
Status: Needs review » Fixed

Fixed a couple of things and committed this.

Anonymous’s picture

Anonymous (not verified) commented

This commit deletes the .module file?

martin107’s picture

martin107 commented

This commit deletes the .module file?

Yes is seemed appropriate.

The functionality in *.module file, the hook functions, have been moved out into various places, for example the info.yml files.

What remains in D8, in module files, is hook_help() functions, plus a few other things.

so we deleted the module file, once it was empty.

damiankloip’s picture

damiankloip commented

Yes, exactly. We have nothing in there now, and there is no requirement in D8 to have a .module file. If we need it in the future, it can come back.

bkosborne’s picture

bkosborne
Primary language English
Location New Jersey, USA
commented
Related issues: +#2887558: Circular reference detected for service "cache.backend.memcache"

Status: Fixed » Closed (fixed)

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

wesleymusgrove’s picture

wesleymusgrove commented

@damiankloip, When I updated from 8.x-2.0-alpha2 to alpha4 I get this error when trying to run drush cr and drush updb:

include_once(/mnt/www/html/example/docroot/sites/all/modules/contrib/memcache/memcache.module): failed to open stream: No such file or directory Extension.php:140

After updating the module and the .module file gets removed, how do I clear the cache so it no longer expects the module file to be there?