search

magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.52k stars 9.31k forks source link

app:config:import stop when retrieve a warning or notice on php code #31428

Closed rubenpurple closed 2 years ago

rubenpurple commented 3 years ago

Hello, im trying to configure the pipeline system of Magento 2.4.

In the production system, the last step, i do the next commands:

/usr/local/bin/php bin/magento app:config:import /usr/local/bin/php bin/magento setup:upgrade --keep-generated

As the documentation says. The problem, is when import the config.php file, i get the next error:

image

looking at the code:

image

The $time variable, get null from function: $this->getData('groups/productalert_cron/fields/time/value');

I changed the code so that if it is null, I left a zero as the value.

Then i try again, and now get the next error: image

When i saw that error is a warning, i try to disable error_reporting in app/bootstrap.php: //error_reporting(E_ALL);

and now, the import works fine. image

I don't understand why I get these errors, caused by the values ​​returned by the functions, which are null.

I checked my database and config.php file, if I have the requested value: image image

now i have the doubt if avoiding error_reporting works fine the import function.

Reggards.

Additional Information

m2-assistant[bot] commented 3 years ago

Hi @rubenpurple. Thank you for your report. To help us process this issue please make sure that you provided the following information:

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release

For more details, please, review the Magento Contributor Assistant documentation.

Please, add a comment to assign the issue: @magento I am working on this

:clock10: You can find the schedule on the Magento Community Calendar page.

:telephone_receiver: The triage of issues happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

:movie_camera: You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

:pencil2: Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

m2-assistant[bot] commented 3 years ago

Hi @engcom-Delta. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

engcom-Delta commented 3 years ago

Hi @rubenpurple thank you for your report, I'm not able to reproduce issue by steps you described on clean 2.4-develop Manual testing scenario:

ini_set('display_errors', 1);

Result: :heavy_check_mark: Import is finished without errors image

Are my steps correct or something was missed?

metadan commented 3 years ago

I have this same issue whilst doing a site upgrade from 2.3.5 to 2.4

rubenpurple commented 3 years ago

Hello, i was talking with my partner, and the magento was 2.3.4 upgrade to 2.4.

Its possible that the error only happens when upgrade? but not with clean new installation of 2.4 version?

barbazul commented 3 years ago

This is actually a very big bug.

AFAIK it affects the following areas:

The issue is that the same value being exported by app:config:dump cannot be read by app:config:imoprt, completely breaking the whole shared configuration deployment pipeline.

So far, the workaround we are using is

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Is this issue still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? Thank you for your contributions!

hostep commented 3 years ago

Leaving a comment to prevent the stalebot from closing this automatically

m2-assistant[bot] commented 3 years ago

Hi @engcom-Bravo. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

m2-assistant[bot] commented 3 years ago

Hi @engcom-Hotel. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

engcom-Hotel commented 3 years ago

Hi @rubenpurple,

We have checked this issue in Magento CLI 2.4.2-p1 with steps mentioned in the portal (https://devdocs.magento.com/guides/v2.4/config-guide/deployment/pipeline/technical-details.html). But the issue is not reproducible. We are going through with below steps:

  1. Start maintenance mode
  2. bin/magento app:config:import to import configuration changes.
  3. bin/magento setup:upgrade --keep-generated to update the database schema and data, preserving generated static files. But the issue is not reproducible. Please let us know if we missed anything.
barbazul commented 3 years ago

Steps to reproduce are the following:

  1. Fresh Magento 2.4.2-p1 installation
  2. Make configuration changes via admin panel (for instance, change store name)
  3. Run bin/magento app:config:dump
  4. Make changes to config.php (for instance, change store name to something different)
  5. Run bin/magento app:config:import
hostep commented 3 years ago

Also ran into this problem today.

Steps to reproduce on latest 2.4-develop:

  1. Setup clean Magento from 2.4-develop branch (I've used commit f121ac83597)
  2. After Magento is up and running, execute bin/magento app:config:dump
  3. Manually change the app/etc/config.php file like this: 
    --- app/etc/config.php.orig   2021-06-22 18:28:35.000000000 +0200
+++ app/etc/config.php  2021-06-22 18:28:42.000000000 +0200
@@ -1302,8 +1302,8 @@
             ],
             'generate' => [
                 'enabled' => '0',
-                    'time' => null,
-                    'frequency' => null,
+                    'time' => '03,07,00',
+                    'frequency' => 'D',
                 'error_email_identity' => 'general',
                 'error_email_template' => 'sitemap_generate_error_email_template',
             ],
@@ -4023,7 +4023,7 @@
                 'jobs' => [
                     'sitemap_generate' => [
                         'schedule' => [
-                                'cron_expr' => '0 0 * * *',
+                                'cron_expr' => '7 3 * * *',
                         ],
                     ],
                 ],
  4. Try to import those changes by using: bin/magento app:config:import
  5. You now get this error in the output:
Processing configurations data from configuration file...
Import failed: Notice: Trying to access array offset on value of type null in app/code/Magento/Cron/Model/Config/Backend/Sitemap.php on line 78
engcom-Hotel commented 3 years ago

Hi @barbazul,

I have tried to reproduce this issue in 2.4.2-p1 and 2.4-develop, using below steps:

  1. Setup and run Magento
  2. Execute bin/magento app:config:dump
  3. Make changes in ./app/etc/config.php as stated by @hostep
  4. Execute bin/magento app:config:import

We are getting below error:

Screenshot 2021-06-23 at 7 41 37 PM
magento-engcom-team commented 3 years ago

:white_check_mark: Confirmed by @engcom-Hotel Thank you for verifying the issue. Based on the provided information internal tickets MC-42706 were created

Issue Available: @engcom-Hotel, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

barbazul commented 3 years ago

@engcom-Hotel Yes I ran into that one as well. Its a different head for the same monster. This one is a little easier to work around as you can actually change the value of the offending config from '[]' to [].

The issue is explained more thotoughly in https://github.com/magento/magento2/issues/12298

waynetheisinger commented 3 years ago

Big issue for pipeline...

github-jira-sync-bot commented 3 years ago

:white_check_mark: Jira issue https://jira.corp.magento.com/browse/AC-988 is successfully created for this GitHub issue.

m2-assistant[bot] commented 3 years ago

:white_check_mark: Confirmed by @engcom-Charlie. Thank you for verifying the issue.
Issue Available: @engcom-Charlie, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

cmuench commented 3 years ago

Big issue for pipeline...

Same here. Gitlab-CI failed. We always to a fresh installation to test the setup process.

cmuench commented 3 years ago

white_check_mark Jira issue https://jira.corp.magento.com/browse/AC-988 is successfully created for this GitHub issue.

@sidolov Not very transparent. How can we see the internals here? Maybe you can share more information.

sidolov commented 3 years ago

@cmuench internal ticket in this case just a copy of GitHub issue. The internal team will pick the ticket according to the priority and all further updates will be reflected here.

cmuench commented 3 years ago

@cmuench internal ticket in this case just a copy of GitHub issue. The internal team will pick the ticket according to the priority and all further updates will be reflected here.

ok. Thanks for clarification. I hope there will be a quality patch for that issue, soon.

zakharevych commented 3 years ago

I fixed the issue with adding of the lines to app/etc/config.php

'frequency' => 'D', 'time' => '00,00,00',

in the 'catalog/productalert_cron' section

engcom-Hotel commented 2 years ago

PR Merged

fredden commented 2 years ago

@engcom-Hotel please can you add a reference to the pull request that fixes this. I'm struggling to find what pull request you're referring to.

hostep commented 2 years ago

I'm going to guess it's solved under AC-988::Fixing app:config:import issue after config dump

But @fredden is correct, next time @engcom-Hotel please provide more info when you close an issue. It's often very helpful for people so they can make patches out of the PR's if they want a quick fix on their shop.

engcom-Hotel commented 2 years ago

@hostep Sure I will take care of the same in the future.

cmuench commented 2 years ago

@sidolov Any chance to publish an official quality patch for that issue?

sidolov commented 2 years ago

Hi @cmuench! You may create a pull request to 2.3.7-release or 2.4.3-release branch to deliver the patch, or use https://github.com/magento/quality-patches/ repo directly

densen45 commented 2 years ago

I experienced that the merge that @hostep referenced in https://github.com/magento/magento2/issues/31428#issuecomment-952079113 solves only part of the issue.

After applying the modifications to the Alert.php and Sitemap.php I get the following error:

Import failed: Notice: Trying to access array offset on value of type null in [..]/vendor/magento/module-config/Model/Config/Backend/Currency/Cron.php on line 66

The changes applied to the Alert.php and Sitemap.php need to be applied to the Cron.php as well.

-            (int)$time[1],                                 # Minute
-            (int)$time[0],                                 # Hour
+            (int)($time[1] ?? 0),                                 # Minute
+            (int)($time[0] ?? 0),                                 # Hour

Has anyone else experienced this?

Aquive commented 2 years ago

@densen45 works, thanks!

In addition I got the following error:

application/vendor/amasty/module-xml-google-sitemap/Model/Config/Cron.php on line 52

The fix is the same:

-            (int)$timeConfig[1],
-            (int)$timeConfig[0],
+           (int)($timeConfig[1] ?? 0),
+           (int)($timeConfig[0] ?? 0),
hostep commented 2 years ago

@Aquive: can you notify Amasty about this and reference this issue in your ticket, so they know what this is about and they can fix it as well on their end, thanks! 🙂

@densen45: could you create a new issue for this here on github? It appears Adobe forgot to fix it for currency rates update cronjob. Also the following cronjobs might still be affected:

Thanks! 🙂

dossy commented 10 months ago

I just ran into this issue with Magento\ScheduledImportExport\Model\System\Config\Backend\Logclean\Cron (vendor/magento/module-scheduled-import-export/Model/System/Config/Backend/Logclean/Cron.php), and found this issue.

Here's a patch for anyone else who needs it:

--- a/vendor/magento/module-scheduled-import-export/Model/System/Config/Backend/Logclean/Cron.php 2023-12-20 02:52:12.287897448 +0000
+++ b/vendor/magento/module-scheduled-import-export/Model/System/Config/Backend/Logclean/Cron.php 2023-12-20 03:05:54.156095976 +0000
@@ -55,8 +55,14 @@
      */
     public function afterSave()
     {
-        $time = $this->getData('groups/magento_scheduled_import_export_log/fields/time/value');
-        $frequency = $this->getData('groups/magento_scheduled_import_export_log/fields/frequency/value');
+        $time = $this->getData('groups/magento_scheduled_import_export_log/fields/time/value') ?:
+            explode(',', $this->getValue());
+        $frequency = $this->getData('groups/magento_scheduled_import_export_log/fields/frequency/value') ?:
+            $this->_config->getValue(
+                'system/magento_scheduled_import_export_log/frequency',
+                $this->getScope(),
+                $this->getScopeId()
+            );

         $frequencyDaily = \Magento\Cron\Model\Config\Source\Frequency::CRON_DAILY;
         $frequencyWeekly = \Magento\Cron\Model\Config\Source\Frequency::CRON_WEEKLY;