dpovshed / octopus

Sitemap checker/stress test tool based on ReactPHP
11 stars 1 forks source link

Prevent use of file_get_contents() #6

Closed holtkamp closed 7 years ago

holtkamp commented 7 years ago

https://github.com/dpovshed/octopus/blob/ec11ba314f17a97d8a8fd27490bfd04dd2410372/src/TargetManager.php#L45

Apparently this is not allowed?

https://twitter.com/Fneufneu/status/890258282362941440

kelunik commented 7 years ago

Of course it's allowed. But it blocks and nothing can happen concurrently. As this is just the bootstrapping, it doesn't actually matter.

dpovshed commented 7 years ago

Guys, while in preparation phase using of file_get_contents is totally fine, I should confess that I also used file_put_contents in the Processor::onData() function.

So if anyone has the desire to speed up file operations, there is still a space for improvement and making the code more 'async-ish'.

kelunik commented 7 years ago

It's local filesystem access there, which is generally pretty fast. It's just fine.

Blocking just means nothing can happen concurrently. It's not really a problem for client applications. It only matters for servers.

dpovshed commented 7 years ago

Thanks @kelunik for your input!

I believe we can close the ticket then.