mxcube / HardwareObjects

**DEPRECATED**: MXCuBE Hardware Objects (now part of HardwareRepository)
1 stars 11 forks source link

set current entry in execution #140

Closed meguiraun closed 8 years ago

meguiraun commented 8 years ago

so we can actually use the get_current_entry method

mguijarr commented 8 years ago

What if an exception happens between set_current_entry(entry) and set_current_entry(None) ? The current entry would not get reset, which is probably wrong ? Same with current_queue_entries.remove(...). I think it should be in a finally clause, or better using a context manager (with keyword argument). What do you think ?

mguijarr commented 8 years ago

I guess we also need a patch for 2.2 right ?

meguiraun commented 8 years ago

ok, I will add that. Once we are all happy with this I will do PR for 2.2

marcus-oscarsson commented 8 years ago

Hi Mikel,

Thanks for the fix, Just a thought, it might be unnecessary to have two different structures for currently executing entry. We currently have self._current_queue_entries and self._current_entry. The current entry should always be the last item in self._current_queue_entries. Which means that get_current_entry() simply could be something like:

current_entry = None

if self._current_queue_entries:
  current_entry = self._current_queue_entries[-1]

return current_entry

Just a suggestion, I don't know if it helps you but it seems like you need the current_entry for something.

Marcus

meguiraun commented 8 years ago

In the beginning I saw the get_current_entry method, and I tried to use it but it was not working since the set_current_entry is never executed. It looked to me more appropiate to to actually call the set_current_entry rather than modifying the existing get_current_entry method. Of course, I am not against your suggestion, it is perfectly fine. Another issue for 2.3?

And by the way... in #385 we retrieve some info from the queue_hwobj, among others which is the entry currently in execution.

marcus-oscarsson commented 8 years ago

Hey,

Okey, its perhaps better to use one way of keeping track of current queue entries. But as you like.

Marcus