cubewise-code / tm1py

TM1py is a Python package that wraps the TM1 REST API in a simple to use library.
http://tm1py.readthedocs.io/en/latest/
MIT License
190 stars 109 forks source link

add_element check if element exists #288

Closed vviau closed 4 years ago

vviau commented 4 years ago

Hi,

TM1py v1.4.1

Currently the function hierarchy.add_element will fail if the element already exists in the hierarchy.

Would it be possible to have similar behavior as DimensionElementInsert function in TM1? i.e. if the element already exists just don't add it to the Hierarchy.

Cheers,

Vincent

rkvinoth commented 4 years ago

Hi @vviau

Why don't you handle that within a try block and parse the response in the except block? That way you can actually make sure that the element has been added.

MariusWirtz commented 4 years ago

Hi,

from a TM1 user perspective, I think you might expect that the operation "fails" silently.

I think as a middle way we could introduce an optional boolean argument to the function that controls if the function throws an error in the case that the element exists already. I just can't think of a good name for this argument 🤔

However, if you add an element as String or Consolidation that already exists as a Numeric it should still throw an Exception in my opinion.

rclapp commented 4 years ago

@MariusWirtz I think this should be handled in the most pythonic way possible. TM1 handles these types of errors pretty poorly. I recommend that we introduce a new type of exception that lines up with the cases where Turbo Integrator would not throw an error. Then we could write code like that

try:
   tm1.element.create(‘new element’, ‘my hierarchy’, ‘my dimension’)
except TM1pyTISilentException as e:
   print(‘TI would be silent about this’)
except:
   print(‘this is a real problem’)
vviau commented 4 years ago

Is it a real problem, trying to insert an element that already exists?

For me, the way DimensionElementInsert function works is quite convenient. It is probably not best practice, you should not try to insert elements if they already exist but it reduces the lines code (being able to avoid adding an if statement each time you are using the DimensionElementInsert function).

As a default, the add_element function should fail silently as TM1 does. If you want to catch this error, we could use a new parameter like error_if_exists=true or debug=INFO.

I think having a similar behavior as TM1 would help TM1 developers to use TM1.

rclapp commented 4 years ago

@vviau Parameters to control error behavior is not a great practice. It's hard to scale it across code and it interferes with the standard ways of error and log handling. TBH TI is the outlier here, code should error if it can't do what you ask it to do.

I am of the opinion that we should do our best to not carry forward the unpredictable patterns of TI. For example, if an element insert operation does not error when an element already exists, why does an element delete operation error if an element doesn't exist?

If you want TI like behaviors you could use unbound TI execution?

scrumthing commented 4 years ago

I agree with @rclapp! Hubert and his colleagues trying there best to get rid of tm1 legacy stuff (aka things that should have been implemented the right way from the beginning) in the rest api. That is a lot of learning for all of us but it makes tm1 better in the future. :-)

vviau commented 4 years ago

alright! It looks like all TM1py gurus do not think that is a good idea.

I was trying to find ways to improve TM1py's adoption to more TM1 developers but maybe we should spend more time explaining how it should work than trying to change the code.

I withdraw my request!

It is great to see that many people caring about the TM1py project.

Thanks for your feedback! I hope you will all continue doingGoodTM1py!