ServiceNowITOM / servicenow-ansible

22 stars 28 forks source link

Idempotency #54

Closed mooky31 closed 3 years ago

mooky31 commented 3 years ago

Hello,

With ansible philosophy of idempotency in mind, trying to update a non-exisisting record should create said record instead of just failing. It is very easy to do so : code for record creation in snow_record.py just need to be moved in the except pysnow.exceptions.NoResults part of modification. Sorry for not submitting a pull request, I don't know how to do it.

n3pjk commented 3 years ago

SNOW's REST API is not idempotent. It exposes tables, not entities. For example, if you say the work_note "Test" should be present, it will add that work_note each time you run the task. The API does not check to see if a work_note already exists with that content. If someone other than ServiceNow were to attempt to write an idempotent module using SNOW's API, they would be at SNOW's mercy in trying to maintain that idempotency, because they are implementing it client-side, not server-side. You could just as easily use the Ansible modules to create an equally idempotent play by querying and then reacting accordingly.

mooky31 commented 3 years ago

SNOW's REST API is not idempotent. It exposes tables, not entities. For example, if you say the work_note "Test" should be present, it will add that work_note each time you run the task. The API does not check to see if a work_note already exists with that content. If someone other than ServiceNow were to attempt to write an idempotent module using SNOW's API, they would be at SNOW's mercy in trying to maintain that idempotency, because they are implementing it client-side, not server-side. You could just as easily use the Ansible modules to create an equally idempotent play by querying and then reacting accordingly.

That's what I did. But I still think the SNOW module whould be a better place to do it rather than the playbook. Isn't actually the module maintainer's job to adapt to the evolution of SNOW API ?

n3pjk commented 3 years ago

Module maintainers are not obligated to provide any utility what so ever. A module that spits out the number 42 regardless of input is just as valid as the most complete and complex solution possible. Such a module, however, would be of little interest to all but the most die-hard of Douglas Adams' fans. If an OEM, such as ServiceNow, provided no API, or an API that only returned 42, we would suffer a similar fate. Thus maintainers are largely constrained by the provided APIs.

If a maintainer happens to actually be the OEM, then those in the Ansible community, who are customers of the OEM, might be able to apply pressure to provide a complete and accurate set of modules. Unfortunately, none of the maintainers of any ServiceNow module, currently in existence, is in the employ of ServiceNow. We maintainers are no more privy to, nor in control of, the inner workings of ServiceNow, or their API, than you are. In fact, my job has nothing to do with writing open source software. I do this voluntarily, first, because it makes my job easier, and, second, because I have a very strongly held belief in the tenets of the open source community, leading me to give back. Others can speak to their own motivations.

That said, community maintainers are really only beholden to the APIs provided by their respective OEMs. Depending on the flexibility of the provided API, it may be possible to create an extended architecture upon them, but realize that, without the direct support of the OEM, that architecture is based solely on supposition and reverse engineering. It is subject to change without warning, and potentially prone to subtle errors. So we try to be as faithful to the APIs as we can.

mooky31 commented 3 years ago

I apologize, "job" wasn't the right word in my answer. By the way I am grateful to the creators of this module, which is provided for free and I find useful.

That said, I still think that it would be smarter to create the record if update fails because of pysnow.exceptions.NoResults. This beahvior is easy to implement, and doesn't change anything in interfacing with the SNOW API. If the maintainer doesn't think so, or even has other things to do, this is fine : he is not my employee.

n3pjk commented 3 years ago

Idempotency requires one to be able to fully express the desired state of an entity, and if that entity is not in that state, make it so, otherwise, do nothing. Let's say you have a record, and you want to set the 'color' field to 'blue'. You need to specify everything about that record, including the fact that the color should be blue, so that, if the record does not exist, it will be created with all of the other fields properly entered. There may even be a need to add child records, or records in lookup tables, to accommodate the creation. This requires a priori knowledge of the entity's schema. While this can be done for specific entities like incident tickets, CIs, etc., it is outside the scope of the API that SNOW has provided, and maintenance would always be reactive. So, while it is easy to determine that the record does not exist, it is by no means a simple, or easily maintainable, matter to create an arbitrary entity given the limited abilities of the API.

mooky31 commented 3 years ago

Indeed you had a broader view than me. I'll go on with my small solutions to small problems :)

Thank you for taking the time to explain.