Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weird behaviour during SEO data retrieval #8

Closed
fmarangi opened this issue Jan 27, 2017 · 4 comments
Closed

Weird behaviour during SEO data retrieval #8

fmarangi opened this issue Jan 27, 2017 · 4 comments

Comments

@fmarangi
Copy link

The method Styla_Connect_Model_Styla_Api::getPageSeoData() is working in a weird way IMHO. If, for some reason, the webservice is not reachable, the method returns an empty array for 5 minutes. I can understand the reasoning behind this choice, but it has very strange consequences.

SEO data is usually cached for a certain amount of time (3600 seconds in our case). The result of the webservice request is stored in Magento cache, so far so good. If one of the requests fails, the already generated cache entries are simply ignored because of these lines:

        if($cache->load('styla_seo_unreachable')) {
            return array();
        }

The service might be temporarily unreachable (actually happens quite often in our case, once every 20 minutes), but even if this is the case, the code should at least check if a cache result is already available for the request before returning an empty array. Instead, one failure prevents all pages from displaying SEO information for 5 minutes. It is extremely important that this information is available for services that parse it, e.g. Facebook. With one error all pages become unusable.

Is the problem know on your side? Can you look into that or should I work on a PR? Thank you.

@Fantus
Copy link
Contributor

Fantus commented Jan 27, 2017

Hi francesco,

thanks for reporting this. i agree with you this would be an improvement for the module. As the changes required should be quiet minimal we already start work on it. I will let you know here when its done and tested.

best
Justus

@Fantus
Copy link
Contributor

Fantus commented Jan 27, 2017

please check out the pull request #9

let us know if you kept that in mind. if its also working for you it will get part of the next release and we merge it into the master

@fmarangi
Copy link
Author

Hi Justus,

thanks a lot for your prompt response. Yes, it looks better now. When are you planning on releasing this? Should I wait a bit or simply patch my code with the fix?

ssachtleben added a commit that referenced this issue Feb 1, 2017
Issue #8: improving the caching of SEO data, in case the API is often…
ssachtleben added a commit that referenced this issue Feb 1, 2017
Issue #8: improving the caching of SEO data
@Fantus
Copy link
Contributor

Fantus commented Feb 1, 2017

just merged and released it v0.1.1.9

@Fantus Fantus closed this as completed Feb 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants