Wasn't sure if this would be content or drupalorg, then found it and MR'd.
Hopefully this is the right queue & version, if not I'll appreciate you correcting it :)
xurizaemon → created an issue.
It would be great if this can happen as part of 🌱 Planning for Release 3.0.0, Drupal 10.3+ Compatibility, Flysystem v3.1 Compatibility Needs work , setting that as parent issue.
xurizaemon → created an issue.
larowlan → credited xurizaemon → .
NW based on feedback from @jonathan_hunt
I support the "delete on error" approach in MR 9785.
Discussed above in 19 is the idea of using a HEAD request to check the file's existence. (I realise I'm responding to a Slack thread from two years ago here, but wanted to put this on record in case the solution turns back from the current MR approach.)
Working recently with Migrate in Islandora and large file ingests (>1GB assets) recently, there's a case where an initial HEAD will not prevent this error: if the file URL is correct and the HEAD is a 200, then retrieval fails for eg timeout or resource limits (server returns a 200, but the content length is incorrect). My recollection is that Guzzle does correctly throw an exception and terminate the stream in this case.
The current behaviour is that the terminated stream will be written as a partial download when the request exits. This MR looks like it would correctly ensure the failed download is removed.
fubarhouse → credited xurizaemon → .
There's more to do here, but I'll move that to a new issue. This one has been around long enough and (highly speculative!) might even discourage smaller contribution efforts.
Thanks to @gabesullice for pushing for progress on this in ✨ Provide a stable release for the v2 branch Active .
In order for a release to be covered by the security advisory policy, it must be stable and supported. Unfortunately, this means that no Drupal 10 compatible release is currently covered.
Appreciate the nudge. FYI there is a release coordination issue in 📌 Release coordination issue for Menu Breadcrumb 2.0.0 Active .
Test run has failed with Github timeouts, I see there's an issue 🐛 Random HTTP timeouts for GitLab CI jobs Active for that mentioned in Slack recently. Will retry once then leave it.
Rebased, back to Needs Review. Thanks all!
Hi @matason, I'm interpreting your comments as indicating your last few commits should be reverted.
I will do that, and your changes will be preserved at https://git.drupalcode.org/project/menu_breadcrumb/-/merge_requests/15/d... - the history of pushes to the MR is visible at https://git.drupalcode.org/project/menu_breadcrumb/-/merge_requests/15/p... should you need to restore something.
(The force push in 5 below will overwrite the state of https://git.drupalcode.org/project/menu_breadcrumb/-/merge_requests/15/c..., but the pipelines tab will continue to record pushes of commits that are now removed.)
The commands I used to do this are:
1. Check out the branch from this MR:
git remote add menu_breadcrumb-3392139 https://git.drupalcode.org/issue/menu_breadcrumb-3392139.git git fetch menu_breadcrumb-3392139
2. Obtain the last commit which we want to preserve from https://git.drupalcode.org/project/menu_breadcrumb/-/merge_requests/15/c... (I used 4cc820aa15b3e0ff2bcb3baa0507912fcd5d7c8a)
3. Rebase the local changes against upstream target branch.
git rebase origin/2.0.x
4. Confirm that there are changes between 4cc820aa15b3e0ff2bcb3baa0507912fcd5d7c8a and the new local state.
git diff 4cc820aa15b3e0ff2bcb3baa0507912fcd5d7c8a..HEAD
5. Force-push the rebased changes
git push -f menu_breadcrumb-3392139 3392139-wrong-breadcrumb-due
If I misinterpreted your intent, you are welcome to restore proposed changes to the MR :)
Thanks @leslieg! Would you care to share a bit more about the creation of the logo? Is it your work or someone else's?
Visually it took me a minute to get the "breadcrumb" angle, but I think it's better than no logo so I'm inclined to merge as-is to support the Project Browser initiative, if someone wants to improve on it in future, that's very welcome too!
xurizaemon → made their first commit to this issue’s fork.
Menu Breadcrumb categories are now: Content Display, SEO, Site Structure.
(This was already the case - maybe someone made that change? Anyway, marking Fixed. If you were asking me to _remove_ the other two categories, please re-open to discuss.)
Thanks all! And thanks @nicxvan for the nudge to land this.
ericgsmith → credited xurizaemon → .
ericgsmith → credited xurizaemon → .
tweaking title for clarity of issue searches :)
hestenet → credited xurizaemon → .
xurizaemon → changed the visibility of the branch 3460401-trivia-typo-fixes to hidden.
xurizaemon → created an issue.
Also occurs when creating a new role. Patch appears to address the issue. Moving to RTBC.
We have a site that is periodically affected by this issue. When investigated, we observe that a single entry in purge_queue table has spiked beyond the 100K limit, which blocks all but manual queue flushes for future operation.
select distinct max(item_id) sample_id, count(*) as count, from_unixtime(min(created)) as min_created, from_unixtime(max(created)) as max_created, data from purge_queue group by data order by count desc limit 10
+-----------+---------+---------------------+---------------------+---------------------------------------------------------------------------------------------------------------------------+
| sample_id | count | min_created | max_created | data |
+-----------+---------+---------------------+---------------------+---------------------------------------------------------------------------------------------------------------------------+
| 9417901 | 1305685 | 2024-05-24 14:15:16 | 2024-06-16 21:46:18 | a:4:{i:0;s:3:"tag";i:1;a:0:{}i:2;s:31:"config:views.view.media_library";i:3;a:0:{}} |
| 9417786 | 4944 | 2024-05-24 14:15:32 | 2024-06-16 21:45:09 | a:4:{i:0;s:3:"tag";i:1;a:0:{}i:2;s:36:"simple_sitemap:example-org-sitemap";i:3;a:0:{}} |
| 9417791 | 4944 | 2024-05-24 14:15:32 | 2024-06-16 21:45:09 | a:4:{i:0;s:3:"tag";i:1;a:0:{}i:2;s:36:"simple_sitemap:example-net-sitemap";i:3;a:0:{}} |
| 9417326 | 4686 | 2024-05-24 14:15:32 | 2024-06-16 21:40:22 | a:4:{i:0;s:3:"tag";i:1;a:0:{}i:2;s:34:"simple_sitemap:example-com-sitemap";i:3;a:0:{}} |
| 9417321 | 4381 | 2024-05-24 14:15:32 | 2024-06-16 21:40:22 | a:4:{i:0;s:3:"tag";i:1;a:0:{}i:2;s:26:"simple_sitemap:example-sitemap";i:3;a:0:{}} |
| 9416566 | 4275 | 2024-05-24 14:15:32 | 2024-06-16 21:35:17 | a:4:{i:0;s:3:"tag";i:1;a:0:{}i:2;s:31:"simple_sitemap:example2-sitemap";i:3;a:0:{}} |
| 9417031 | 188 | 2024-05-26 20:36:28 | 2024-06-16 21:38:15 | a:4:{i:0;s:3:"tag";i:1;a:0:{}i:2;s:9:"node_list";i:3;a:0:{}} |
| 9404071 | 176 | 2024-05-26 02:01:34 | 2024-06-16 19:45:47 | a:4:{i:0;s:3:"tag";i:1;a:0:{}i:2;s:19:"config:webform_list";i:3;a:0:{}} |
| 9404076 | 176 | 2024-05-26 02:01:34 | 2024-06-16 19:45:47 | a:4:{i:0;s:3:"tag";i:1;a:0:{}i:2;s:23:"webform_submission_list";i:3;a:0:{}} |
If others are observing this issue, I'm interested to know if executing the query above on their site reveals a similar profile - ie that when grouped by data column, the entries in purge_queue are heavily dominated by a single value of data.
What is the latency that you experience if you use purge_processor_cron?
@D34dMan I don't understand this question, sorry, but I'll try to answer anyway :)
I don't understand how "latency" is being used in the module documentation. I would not consider a site hosted on a managed platform, eg Acquia's, to be "high latency". If asked to define "high latency" I'd probably say "network round trip time over 250ms", which would hopefully exclude any professional hosting environment.
However, it doesn't make sense to me to think of latency as ping time here, unless such latency was internal to the hosting environment. So I'm my conclusion is that I don't know how the writer is using "high latency" in this context.
My approach for the one site we have using Acquia Purge is pretty much as Dan162 suggests, though I hadn't yet investigated the Purge Queues module.
This issue is really about the guidance provided via module documentation, Acquia's docs, and the status report warning displayed. To me the guidance seems to conflict (as noted in ID above), and I am hoping that it can be clarified.
My interest in understanding the recommended configuration is because of a site which has some issues which appear to relate to purge queues, detailed here (no issue link as yet; the cause seems to be Acquia Cohesion, which does not appear to have an issue queue). When we enabled the Late Runtime Purge Processor, we observed negative impact on site performance, and it did not process purge queues any better than cron operation. So we disabled it again.
Thanks both for your input! It's appreciated.
I agree that there's a use case for a "null-ish" destination - I often use something like Gold's Noop extension of core's Null destination plugin from #11 above if I want to debug a process plugin on the CLI.
The Migrate Sandbox → module might be another option, but to me it makes sense to have the option of a destination plugin that permits debugging sources and processes without needing any destination configuration or resulting in local content output.
Anyway, I'm here to +1 the idea of making this destination a living thing, if that works!
fubarhouse → credited xurizaemon → .
fubarhouse → credited xurizaemon → .
@tariqDev I suggest to try the patch available from https://git.drupalcode.org/project/drupal/-/commit/e1fc76a1d2d9ff925bdbb... which may apply fine to your 10.x site.
Thanks LeDucDeBleuet! Yeah I was assisting someone in Slack and they were referencing this issue in trying to apply the patch.
After a closer read I see that in comment 49 🐛 Media Library Theme Reset not supported Needs work @Rajab decided
Decided to only remove the media_library_theme_reset:media_library_theme_reset
Developers in projects can manage any type of modals on their own way.
I am hiding the patch from 31 (since now that #49 was committed, that patch doesn't apply).
I do believe the patch attached in #2 correctly addressed an issue affecting requests for /user/register.
I am not sure about the additional MR content though, and worry that marking the issue RTBC might appear to be +1ing the MR. So I'm just leaving a comment to say, the patch in #2 fixed a problem for us :)
Updating title since patch proposes to add a replacement rather than just removing
JCT321 → credited xurizaemon → .
JCT321 → credited xurizaemon → .
JCT321 → credited xurizaemon → .
JCT321 → credited xurizaemon → .
JCT321 → credited xurizaemon → .
JCT321 → credited xurizaemon → .
JCT321 → credited xurizaemon → .
JCT321 → credited xurizaemon → .
JCT321 → credited xurizaemon → .
JCT321 → credited xurizaemon → .
JCT321 → credited xurizaemon → .
Moving this back from RTBC since there are changes to be reviewed!
@zenimagine, on this issue a Merge Request ("MR") has superseded the patch submitted in #2. So review should focus on the changes available from the MR. On the MR URL, you'll see tabs for eg "Commits" (the initial change from #2, and later changes) and "Changes" (the current proposed changes from this MR).
The required patch is linked on this issue (text "plain diff" near MR !15), which is the MR URL with .diff appended.
That file can be saved and used as a patch in your local codebase, or (quicker but not recommended for production builds) referenced directly in your composer patches configuration.
@zenimagine, here are docs for patching modules when using composer →
hestenet → credited xurizaemon → .
hestenet → credited xurizaemon → .
xurizaemon → created an issue.
Thanks Manish! Yes, please do open a separate issue for the item noted in #10.
There is a Menu Breadcrumb fix related to route parameter matching in 🐛 Wrong breadcrumb due to route alterations RTBC which might be of interest. Thanks for linking #2875276: Breadcrumbs disappears when starting with front-page after cache rebuild (for anonymous user). → as well Nic.
Hi and thanks again!
Please restore the removed items (can be sourced from new issue → ) from the new issue template (changes, tasks remaining etc). Those are worth keeping even if it feels not relevant (eg "no user interface changes are expected") rather than omitted/implied.
Please add to tasks remaining: test coverage, for which I hope ✨ Add option to hide single breadcrumb Needs work has a good reference to start.
@dgroene I've added some additional tests which hopefully will help to clarify the expected behaviour. Please do continue working on this if you can!
Take a look at https://git.drupalcode.org/issue/menu_breadcrumb-3269452/-/blob/3269452-... (and other tests in that directory) to see a test which can cover this behaviour. I'm sure it can be improved, hoping to give you a good start point - let's not presume it's 100% correct please :)
Thinking this might be better labelled as "Remove single items", to match the existing "Remove home" setting?
(I find that when I encounter a setting phrased negatively (eg "Do not add single item"), and I see that the setting is currently FALSE, I can hear clapper boards flipping in my head as I try to calculate that if-not-do-not-add means do or don't do X. This is likely a me problem, but I think it can improve both comms and DX generally to prefer positively phrased statements!)
Attaching interdiff to show changes between previous two patches.
Let's switch to using Gitlab MRs over patches if you don't mind? ( Using Gitlab to contribute to Drupal → .)
Let's also add this to the existing test coverage in tests/src/Functional/SettingsFormTest.php please.
xurizaemon → made their first commit to this issue’s fork.
Thanks for the patches!
We can consider this if the changed behaviour is opt-in. The existing patch looks like it implements the change you desire, but that would affect all sites using the module, and this won't be what many other sites using the module would anticipate or desire.
If implemented, this will need both a config option to enable the new behaviour, and the default behaviour should be the current behaviour. There should also be test coverage, which I can assist with.
I have updated the issue description with some more complete details and remaining tasks.
On the next code submission to this issue, let's please switch to using a merge request? (That's a request, patches are still welcome of course but MRs are preferred for collaboration and community process.)
@manish-31, thanks for self-assigning this issue :)
New features will require test coverage to be accepted. Before starting, please take a look at the tests WIP in the following issues:
-
🐛
Wrong breadcrumb due to route alterations
RTBC
-
📌
Add automated test coverage
Needs review
(two MRs, one is WIP)
If you have any questions, sing out!
Take a look at ✨ Allow taxonomy attachment for any content entity type Needs review which may be relevant?
This will need simpler steps to reproduce, or a test case to establish the incorrect behaviour.
Feature contribution is welcome :)
Happy to consider it for 2.0.x series, needs re-roll and test coverage please.
Needs test coverage and/or steps to reproduce.
There's an option in the settings "Show current page as link".
No info provided, closing.
Closing a lot of ancient (> 4 years) issues. It's fine to re-open if you think there's something of value to be discussed.
Closing a lot of ancient (> 4 years) issues. It's fine to re-open if you think there's something of value to be discussed.
Closing a lot of ancient (> 4 years) issues. It's fine to re-open if you think there's something of value to be discussed.
Closing a lot of ancient (> 4 years) issues. It's fine to re-open if you think there's something of value to be discussed.
Closing a lot of ancient (> 4 years) issues. It's fine to re-open if you think there's something of value to be discussed.
Closing a lot of ancient (> 4 years) issues. It's fine to re-open if you think there's something of value to be discussed.
Closing a lot of ancient (> 4 years) issues. It's fine to re-open if you think there's something of value to be discussed.
Closing a lot of ancient (> 4 years) issues. It's fine to re-open if you think there's something of value to be discussed.
Closing a lot of ancient (> 4 years) issues. It's fine to re-open if you think there's something of value to be discussed.
Closing a lot of ancient (> 4 years) issues. It's fine to re-open if you think there's something of value to be discussed.
Closing a lot of ancient (> 4 years) issues. It's fine to re-open if you think there's something of value to be discussed.
Closing a lot of ancient (> 4 years) issues. It's fine to re-open if you think there's something of value to be discussed.
Thanks! Some further discussion & notes on 🐛 Wrong breadcrumb due to route alterations RTBC already (and a test now).
Seems this can be repro'd as easily as taxonomy/term/1 - debug here from \Drupal\menu_breadcrumb\MenuBasedBreadcrumbBuilder::addMissingCurrentPage() shows that route and raw parameters may be sorted differently. Added test \Drupal\Tests\menu_breadcrumb\Functional\MenuBreadcrumbTest to cover this.
Credits transferred, closing duplicate. Head on over to 🐛 Wrong breadcrumb due to route alterations RTBC :)