search

green-coding-solutions / green-metrics-tool

Measure energy and carbon consumption of software
https://metrics.green-coding.io
GNU Affero General Public License v3.0
159 stars 22 forks source link

Move `set_blocking` to a watcher #151

Open ribalba opened 1 year ago

ribalba commented 1 year ago

Currently we set os.set_blocking(self._ps.stderr.fileno(), False) at the end of the start_profiling method. As seen today this might cause problems if people extend the method as it could be forgotten. I would suggest to wrap it into a watcher and set it as soon as _ps is set. Also we don't need to think about adding it when overwriting the start_profiling. Ideas?

ribalba commented 1 year ago

@ArneTR @djesic @dan-mm Feedback welcome.

ArneTR commented 1 year ago

The process must exist in order to make this call.

Typically the process is just created in start_profiling.

When this method is overwritten this indicates that you do not want to leverage the standard process launcher mechanism of the class and thus do not need its handling in terms of blocking and killing.

We have a refactoring coming up that allows to more flexibly use the start_profiling class by giving the process-filename to invoke in the class variables.

I guess I would leave the set_blocking call in the base functionality then. or?

ribalba commented 1 year ago

Somehow network was slow in the train and my old version got posted. I noticed this needed to be solved differently half way through typing the issue. But you are right in that if someone overwrites the method they might want it to be blocking.