Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Since the hook_cron inside honeypot checks that the css file is in it's place and not its contents those may get outdated when you change the element.
I think is not such an expensive operation to rebuild this css file on cron everytime.
I'll wait for feedback before rolling the patch that removes those two lines.
Right now what I'm doing is removing the file on a hook_update_N
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | rebuild_css_when_the-2608060-8.patch | 3.04 KB | geerlingguy |
| |||
Comments
Comment #2
geerlingguy CreditAttribution: geerlingguy as a volunteer commentedDoesn't it get rebuilt when you save the Honeypot admin form?
Otherwise, we could check the CSS file to make sure the class is the one that's specified in the variable (that way you could change the variable outside of the admin form itself, e.g. through settings.php,
drush vset).Comment #3
rodrigoaguileraExactly, I'm using it through strongarm + features
Comment #4
geerlingguy CreditAttribution: geerlingguy as a volunteer commentedChanging to a feature request, and updating the title. I think this should be doable.
Comment #5
jastraat CreditAttribution: jastraat at Technivant commentedI would also be interested in this. It's useful to be able to periodically update this variable with drush in all sites within a multi-site install.
Comment #6
rodrigoaguileraI haven't thought about that, maybe making this variable an array and cycling through a list of names could add more effectiveness to the module but that should be on another issue.
Comment #7
geerlingguy CreditAttribution: geerlingguy as a volunteer commentedQuick benchmark:
This is on an NFS mount, so times would be slightly faster with a local SSD, slightly slower depending on other factors... It's not the end of the world, but I don't take lightly an operation that adds a ton of extra time in performing a frequently-run operation...
That being said, we're talking < 100ms even in less-than-ideal situations like a slow GlusterFS mount, so I'm going to do a couple quick checks into other techniques, then if those don't pan out, I'll just pull out the check for the file existing in
honeypot_cron().Comment #8
geerlingguy CreditAttribution: geerlingguy as a volunteer commentedGoing to do in 7.x first, as I was already playing around with the code there. Will forward port after new tests are green.
Also, this patch includes a new function
honeypot_check_css(), which only takes 0.3-0.5ms to read the file and scan the single line for the element name (usingstrpos()). Just because I hate adding in unnecessary write operations to something that would potentially run up to once per minute on certain sites, I think it's worth adding a simple extra function to check the file before writing a new copy.So basically, the goal of this ticket is resolved by the file only being written if either (a) it doesn't already exist (0.1ms lookup) or (b) it exists but doesn't contain the correct honeypot_element_name (0.5ms lookup). The 4+ms write operation only takes place if both a and b checks fail.
Comment #9
geerlingguy CreditAttribution: geerlingguy as a volunteer commentedD7 commit: http://cgit.drupalcode.org/honeypot/commit/?id=f74e015
On to 8...
Comment #10
geerlingguy CreditAttribution: geerlingguy as a volunteer commentedThis is not an issue in 8.x, since the HTML is attached as a separate
<style>tag in the head.