πŸ‡ΊπŸ‡ΈUnited States @dpagini

Account created on 8 February 2012, over 12 years ago
#

Merge Requests

Recent comments

πŸ‡ΊπŸ‡ΈUnited States dpagini

Marking this as fixed by https://www.drupal.org/project/memory_limit_policy/issues/3448480 πŸ“Œ Investigate config validation Active

πŸ‡ΊπŸ‡ΈUnited States dpagini

Just revisiting this one, and my comment in #13... I suspect that for my issue, this was somehow related to using `advagg` module, and when Drupal made the aggregation changes in 10.1, I believe, we stopped using advagg and I have not seen the issue again since we switched.

πŸ‡ΊπŸ‡ΈUnited States dpagini

dpagini β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States dpagini

Also huge thanks to @fjgarlin who helped me out in the Portland General Contribution room. Apologies I should have given that credit as well!

πŸ‡ΊπŸ‡ΈUnited States dpagini

Assigning partial credit to jrockowitz who put up test changes(fixes) in https://www.drupal.org/project/config_override/issues/3023903 ✨ Allow config override directory to be stored outside of Drupal. Needs review that I used to get tests to pass in the new Drupal CI.

πŸ‡ΊπŸ‡ΈUnited States dpagini

dpagini β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States dpagini

This test is not reflective of how the configuration behaves in the real word, because the configuration should never end up like this if it was saved via \Drupal\language\LanguageNegotiator::saveConfiguration

I'm not sure what this is supposed to mean, @larowlan? I have run into the same issue described in this IS, and it seems a few others on this thread have as well... is that not the real world evidence? Why should there be a configuration, language.types, that is NOT overridable, the way every other config in Drupal core is...? How could a developer possibly know that for this one special config, you can't override it, and instead have to call the LanguageNegotiator::saveConfiguration()?

I'm not sure what the opposition to introducing a fix here that allows you to set this with configuration overrides would be? "It wasnt intended to do that" seems like a very poor reason...

I agree with larowlan, that this use case is not covering a real world scenario. It does not use the correct methods. Until now LanguageNegotiator was not intended to be configured by direct configuration changes or in settings.php.

So a user of Drupal is only allowed to override some configuration...?

I don't think that just changing the comment as suggested in the most recent patch/MR actually solves the bug that was described in the issue summary.

I agree completely.

πŸ‡ΊπŸ‡ΈUnited States dpagini

I want to set this to "Needs review", maybe even RTBC... I don't know why, but I can't set one of these patches to test against the 2.x branch, so it shows that it fails to apply to 1.x. I am using the 1.x version of the patch in my project, and it is solving this issue for me... I'm curious if this task has some actionable steps now to document?

πŸ‡ΊπŸ‡ΈUnited States dpagini

Yep, I'm with you @capservoogt. This has been a very difficult issue for us to watch Acquia leave us up the creek without a paddle. I am trying to use what code we already have and patch something together for D10 with a plan to move off this tool as soon as is feasible.

πŸ‡ΊπŸ‡ΈUnited States dpagini

I've actually recently run into this issue, and I have some more details that affected me here I wanted to share...

The fix here still solves my problem, and I can explain why as well.

So I ran into this issue with the following scenario...
* I am installing a site that uses a custom cron job to make a call out to an RSS feed, download the file, and store it locally for processing by the feeds module.
* Drupal core runs through the INSTALL steps... but the install_configure_form step actually hardens the simpletest site directory, and sets the folder to 555 (not writeable).
* Drupal core at the end of the installation then triggers a cron. My job calls out to the internet. Drupal core has a custom testing middleware that intercepts that calls, and tries to add testing headers to the call, and ultimately calls drupal_generate_test_ua(), which tries to write the .htkey file to a hardened sites directory, and that fails... so my first cron run fails silently.

I actually came across this b/c by enabling the "locale" module, the call to drupal_generate_test_ua() was made earlier in the stack, before the hardening, and the .htkey file gets placed successfully, and there is no more failure.

πŸ‡ΊπŸ‡ΈUnited States dpagini

The last 2 comments are saying the opposite... could we maybe get more elaboration on #103? I'm using this patch with Claro and it is resolving this issue, so I'm going to put to RTBC, but with the caveat that I'm not testing in Gin and 103/104 are saying it's working/it's not.

πŸ‡ΊπŸ‡ΈUnited States dpagini

What do you mean by that, @smustgrave?

πŸ‡ΊπŸ‡ΈUnited States dpagini

I came across this other core issue regarding these same links, and I think a solution should probably consider both of these issues...?

πŸ‡ΊπŸ‡ΈUnited States dpagini

Interested in this as well... seems related to 2990549, although not exactly the same thing.

πŸ‡ΊπŸ‡ΈUnited States dpagini

When reading through this issue, I think this should ONLY be tagged for branch 9.5.x, which is about to (3 months) be marked unsupported, potentially killing this issue. But yeah, I don't think this is relevant to 10.x or 11.x at all, since they are using the new 2.x asm89/stack-cors library. Does anyone disagree with that?

πŸ‡ΊπŸ‡ΈUnited States dpagini

I'm going to bump this RTBC to get some more eyes on this. I have been using this allowedOriginsPatterns in production for the better part of a year, with a note in my codebase that this is undocumented in Drupal core. I think this should go into core to let people know it's an available option for them.

πŸ‡ΊπŸ‡ΈUnited States dpagini

@nterbogt Could you see if this problem still exists on the latest version of the 2.x branch?

πŸ‡ΊπŸ‡ΈUnited States dpagini

Thanks all. Sorry this took so long to get in, I have a really terrible excuse I won't share here out of embarrassment.

πŸ‡ΊπŸ‡ΈUnited States dpagini

Patch applied to the dev branch, will look at creating a new release soon.

πŸ‡ΊπŸ‡ΈUnited States dpagini

Thanks for your contribution, sorry for the delay, but I hadn't noticed this issue created before now. I am seeing the issue here that this was deprecated in 9 (so still working), but is now breaking in D10.

πŸ‡ΊπŸ‡ΈUnited States dpagini

Confirming this issue is fixed in the latest `dev-5.x` branch. Thank you all!

πŸ‡ΊπŸ‡ΈUnited States dpagini

I'm still seeing failures with this commit. It looks to me like there's an indentation issue in the YML file. I would recommend setting up a very basic automated test in the module that enables a test module that has this config present, and then you should be able to see the schema errors for yourself.

πŸ‡ΊπŸ‡ΈUnited States dpagini

I actually am running into a very similar issue, but I don't know how to recreate. My production site will occasionally have a page that does not load javascript... It has 1 aggregated file that's supposed to be there, but instead my page prints: <script type="text/javascript" src="/"></script> which points to the homepage, and is HTML and not js, so I have no scripts on my page. I do use the advagg module, so not sure if that's related...?

πŸ‡ΊπŸ‡ΈUnited States dpagini

I have also encountered this error (or similar) a few times on my production site. I haven't been able to determine if it's related to this module, though, but I am using this module, still on Drupal 9.
On my site, however, I am seeing that Drupal is loading a script at <script type="text/javascript" src="/"></script> (the homepage).

πŸ‡ΊπŸ‡ΈUnited States dpagini

Just want to say thank you @Berdir and all involved!

πŸ‡ΊπŸ‡ΈUnited States dpagini

A BIG issue I'm seeing here is that `drupal/media_acquiadam` requires `cweagans/php-webdam-client`, which is archived (abandoned), and requires `guzzlehttp/guzzle` < 7.0, but `drupal/core:^10` requires `guzzlehttp/guzzle` ~7.5.0.

πŸ‡ΊπŸ‡ΈUnited States dpagini

The error I get is something along the lines of lockr.lockr_secret._____:info.wrapping_key missing schema. In the code in your screenshot, you are setting `info` to a sequence, but then not defining that sequence. I don't think you want to define this key as a sequence, but instead as a mapping, with the corresponding keys (wrapping_key) then defined.
Did you get the file that I sent over to Chris T? That has the changes that are working for our team, but I don't think you want the patch exactly.

πŸ‡ΊπŸ‡ΈUnited States dpagini

Hey Wim - I see this has been up here for 6+ months, but I hadn't noticed it until now, apologies. I'd be happy to help test out a proposed fix, but I see that this is failing for maybe a PHPUnit rule to make sure that \Drupal\Core namespace does not get called from \Drupal\Components...? Given that rule, a fix like this wouldn't be able to go in, would it? If you'd like, I can still try this fix against my project's problem and see if that solves the issue if it helps move this along...?

My team is currently using a custom version of the work-around Alex suggested, and that has been working for us.

πŸ‡ΊπŸ‡ΈUnited States dpagini

@heddn - they manage externally at https://github.com/pantheon-systems/search_api_pantheon - does that help at all?

This did the fix the issue for my team. Isn't the `skip_schema_check` reference coming from here...?
https://git.drupalcode.org/project/search_api_solr/-/blob/4.x/config/sch...

πŸ‡ΊπŸ‡ΈUnited States dpagini

Suggesting this issue gets closed...? There is an RC release out now for this module...

I came here looking for a roadmap for a stable release, and one that's covered under the Drupal security policy...?

πŸ‡ΊπŸ‡ΈUnited States dpagini

Given the last 3 comments, changing this to RTBC status. My team is using this patch as well and found that it fixed the issue we were facing.

πŸ‡ΊπŸ‡ΈUnited States dpagini

For me, this is an automated testing only issue. When you run PHPUnit tests, part of the testing procedure ensures that all config you are importing has a valid schema, and since the schema being provided by this module is being reported as invalid, when using this module as part of an automated test, the test always fails.

So as an example, if I have a my_module.info.yml, and in my dependencies of the module, I list search_api_pantheon, and then I have a Functional test in my module, my test will fail 100% of the time.

So to reproduce, you can probably just add a FunctionalTest to your project, and that should start to fail.

I also don't think you need all of what is included in the latest patch here. The PR that's open on the Github.com repo (github.com/pantheon-systems/search_api_pantheon/pull/169/files) actually solves the problem for me.

πŸ‡ΊπŸ‡ΈUnited States dpagini

Definitely +1 on this issue and following what happens here...

It's a bit challenging to test this with changes needed to the composer.json file, right? You can't apply a patch for this before composer runs...? How are folks testing this out?

My team is also "trapped on Acquia DAM Classic" (to borrow a phrase from a previous comment). We have been advised by Acquia that there will be no more releases for this 1.x branch, even for Drupal 10 compatibility. I'm not entirely sure if that would extend to reviewing and merging a patch request such as this, but just wanted to give the folks who came here for this a heads up.

If Acquia will not review this approach and move it forward, I'm wondering what folks see as the option here to use this module on Drupal 10?

πŸ‡ΊπŸ‡ΈUnited States dpagini

Can this be reviewed for Lockr 5.x if the same config objects are supported in that branch?

πŸ‡ΊπŸ‡ΈUnited States dpagini

This patch (not exactly) is already applied to the `5.x` dev branch, but there's no release tied to that branch yet. I have been using 5.x in a lower environment, and everything is working as expected for me.

πŸ‡ΊπŸ‡ΈUnited States dpagini

This patch now no longer works for me...

The recent commit to this project added a schema, but it does not appear to be correct. This commit attempts to define every property of the solr connector, whereas the patch in #7 in this issue was extending the plugin.plugin_configuration.search_api_solr_connector.standard schema.

So now, when I run this project, I get the following error...

Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for search_api.server.pantheon_solr8 with the following errors: search_api.server.pantheon_solr8:backend_config.connector_config.solr_version missing schema, search_api.server.pantheon_solr8:backend_config.connector_config.http_method missing schema, search_api.server.pantheon_solr8:backend_config.connector_config.jts missing schema, search_api.server.pantheon_solr8:backend_config.connector_config.solr_install_dir missing schema

In my opinion, the commit that was merged to the project (linked above) should be reverted, and the patch from #7 above should be committed in it's place.

πŸ‡ΊπŸ‡ΈUnited States dpagini

I see that a schema was merged in as part of the v8.1.3 release, but it is not the same as the patch here, and the published fix does not resolve my schema errors, but the patch does.

πŸ‡ΊπŸ‡ΈUnited States dpagini

I was running into this same issue, pulled in this patch, and now my tests are passing again. I think this should be added into this module! Thanks @heddn!

πŸ‡ΊπŸ‡ΈUnited States dpagini

+1 for this patch, it solves the same problem I'm facing... Url's in the "From" field with a special character, like `path/to/a%20file.pdf` won't work.
This issue has been RTBC for almost a year now, is there anything left the maintainers need on this issue? I see that, it appears, the last requested changes by Berdir were completed, so I think this just needs eyes again from an maintainer.

πŸ‡ΊπŸ‡ΈUnited States dpagini

This patch fixes a very narrow use case of a space being encoded to %20, but does not address all encoded characters. For example, a ( or ) get encoded to %28 and %29. That makes me think there should be a more global solution to handle this...?

πŸ‡ΊπŸ‡ΈUnited States dpagini

It looks like this was a duplicate of #3322852, although maybe that issue should have been marked a duplicate of this one. Thank you for your contribution!

πŸ‡ΊπŸ‡ΈUnited States dpagini

Are there any reproduction steps that can be provided here? How can I set up a test to see this fail for myself?

πŸ‡ΊπŸ‡ΈUnited States dpagini

I did see that, but did not try it...
Ok so this is working for me now again. Sorry for all my confusion. I think the issue was the invalid tokens (using Gutenberg, it saves image style tokens to the source HTML) that were causing the styles to not generate.

πŸ‡ΊπŸ‡ΈUnited States dpagini

Ugh, sorry... I thought this was fixed, but I still don't think it is. It was just working locally, but that was still b/c I was visiting the page in a browser, which creates the image styles.

πŸ‡ΊπŸ‡ΈUnited States dpagini

Actually, I don't think this was an issue. My problem, I believe, was somehow (not sure yet how) my pages were referencing image styles with invalid tokens. Since I'm using a static site, I don't really care about the image style token, so I can shut that off in the image.settings configuration.

πŸ‡ΊπŸ‡ΈUnited States dpagini

dpagini β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States dpagini

Seems like this should be removed to me... and a 6.x branch in pre-release seems like a great opportunity to do so. It could also probably be marked as deprecated in the 5.x branch to warn users ahead of a 6.x upgrade. Just my $0.02.

πŸ‡ΊπŸ‡ΈUnited States dpagini

Any chance this can be backported to 5.x?

Production build 0.69.0 2024