Timezones in cron will change between Magento 2.1.x and 2.2.x, how to handle this change?

[hostep]

Hi guys

Recently a Pull Request: magento/magento2#9943 was accepted and it will be released in Magento 2.2.0: magento/magento2#4237 (comment)

This fixes an issue with wrong timezones being used when inserting and reading times in/from the cron_schedule table.

The problem is, that n98-magerun2 also has some functionality which uses the wrong timezones for running and scheduling cronjobs:

• https://github.com/netz98/n98-magerun2/blob/c7ac113/src/N98/Magento/Command/System/Cron/RunCommand.php#L54-L68
• https://github.com/netz98/n98-magerun2/blob/190b2d4/src/N98/Magento/Command/System/Cron/ScheduleCommand.php#L44

So I think this bug should get fixed in magerun2 as well.
The question is: how will this be handled? Since magerun2 will be used both in Magento 2.1.x versions as well as Magento 2.2.x versions.

Should we check for the current version of Magento in use and then change the behavior of the code, or will magerun2 create a new version which won't be backwards compatible anymore with Magento 2.1.x for example?
Also: maybe the above PR will get backported to Magento 2.1.x one day, it might happen...

These are just some things which popped into my head.

Thanks!

[ktomk]

Thanks for pointing this out!

If I understood you right, it should be enough to use the correct time-stamp string values.

So if timestamp creating is extracted and moved into an abstract method in the base class, the decision on version can be implemented there.

$this->timezone->scopeTimeStamp() -> $this->getTimeStamp()

And then changing AbstractCronCommand::getTimeStamp() according to the needs (check magento version to branch timestamp creation).

In case this is backported, all that changes is the magento version to compare against.

This would be quite straight forward for the changes.

It would be much greater however if there are classes / model in Magento 2 which would allow to to the run / shedule cron operations in a more API friendly way. In such a case we would automatically benefit from upstream changes. But I smell that these are not available. Last time I looked (ca. 4-5 month ago) cron handling code was pretty spread out and there was little to no API for it.

[hostep]

This sounds like a good solution :)

API-wise, I doesn't look like much has changed unfortunately, most of the useful functionality is part of the ProcessCronQueueObserve class, which only has one public method.

[ktomk]

That's what I thought of, this one big Moloch handling "all the things" :)

Anyway thanks again for report and review, I just pushed some changes that should handle it: 854d598...2485021

---

We believe that the bug reported is fixed in the development version. It can be upgraded to it using the self-update command with the --unstable switch.

[hostep]

Wow nice one, this went a lot quicker then I expected, tnx! :)