Hi! I've thought about this in a slightly different way. I'm more used to the more stringent definition of idempotency that I learned very-long-ago in an OS class. "An operation is idempotent when the effect of performing it multiple times is equivalent to performing it once." I pulled it from a recent textbook by the professor, http://pages.cs.wisc.edu/~remzi/OSTEP/dist-nfs.pdf, I don't still have my notes or anything like that. So, I do think we would be giving up the idempotency that we've been striving for in the CLI if we make the proposed changes (by that definition of idempotency). But there is also the consideration of user expectation, which is important. Therefore, giving up idempotency in the CLI, in order to better accommodate user expectation might be the correct thing to do. It will, perhaps, open up new problems for users who script the CLI, however, so we should be aware of that possibility. Retaining idempotency in the D-Bus API is still a desirable goal. If we issue a sequence of commands on the D-Bus and the D-Bus server suffers a failure, then, if those commands are idempotent, we are free to issue them again. This would be a good thing. My concrete suggestion is the following: 1. Fix the bug in the "filesystem create" command. If there is already a filesystem there, have it return success on the D-Bus, but an empty list of filesystems created. NOTE: StratPool::create_filesystems() should be defined so that it creates any filesystem that does not have a conflicting name. It is implemented to create multiple file systems. It is only called w/ one filesystem spec, though, so always, under this situation if there is a name conflict it should return Ok(vec![]). 2. For each CLI call where the idempotency behavior conflicts with user expectation, fix it up in the CLI. These are: 1. rename for pool and filesystem 2. destroy for pool and filesystem 3. create for filesystem (Not for pool, because create for pool seemed too hard to make idempotent. Note that, according to my textbook, just because you can't meet the goal of idempotency in every case, that doesn't make useless the cases where you do. Even in NFS, mkdir is not idempotent. We should make sure one last time that create pool is too hard to make idempotent, though, before we leave it for good.) How to fix it up in the CLI? Take the function create_volumes, assuming that the D-Bus return has been corrected in the manner I describe above as an example. @staticmethod def create_volumes(namespace): """ Create volumes in a pool. :raises StratisCliEngineError: """ proxy = get_object(TOP_OBJECT) managed_objects = ObjectManager.Methods.GetManagedObjects(proxy, {}) (pool_object_path, _) = next( pools(props={ 'Name': namespace.pool_name }).require_unique_match(True).search(managed_objects)) (_, rc, message) = Pool.Methods.CreateFilesystems( get_object(pool_object_path), {'specs': namespace.fs_name}) if rc != StratisdErrors.OK: raise StratisCliEngineError(rc, message) Get the list of object path/name pairs returned by the CreateFilesystems call. If the names returned does not match the requested names, raise an appropriate exception. I expect that a new type of exception would have to be created for this purpose. Field the exception appropriately in the run method and report the error. This way, we retain idempotency as much as possible in the D-Bus API but meet user expectations in the CLI. - mulhern ----- Original Message ----- > From: "Jakub Krysl" <jkrysl(a)redhat.com&gt; > To: stratis-devel(a)lists.fedorahosted.org > Sent: Tuesday, March 19, 2019 11:08:41 AM > Subject: [stratis-devel] idempotency versus/and user comfort > > Hello all, > > Recently I've raised an issue about user comfort when destroying stratis fs > [1] which got rejected with idempotency as reason. I do not agree with this, > let me explain why. > > First let's start with the user comfort itself. It seems to be good practice > to fail on item removal if the item user is trying to remove is not present. > This can have many reasons varying from "item is already removed" to "user > made typo in item name". Here are some examples: > device mapper: > # dmsetup table > test: 0 102400 linear 7:0 102400 > # dmsetup remove test > # echo $? > 0 > # dmsetup remove test > device-mapper: remove ioctl on test failed: No such device or address > Command failed. > # echo $? > 1 > LVM: > # pvremove /dev/loop0 > Labels on physical volume "/dev/loop0" successfully wiped. > # echo $? > 0 > # pvremove /dev/loop0 > No PV found on device /dev/loop0. > # echo $? > 5 > > On the other hand Stratis CLI does not work this way: > # stratis pool create test /dev/loop0 > # stratis fs create test test_fs > # echo $? > 0 > # stratis fs create test test_fs > Execution failure caused by: > ALREADY EXISTS: test_fs > # echo $? > 1 > # stratis fs destroy test test_fs > # echo $? > 0 > # stratis fs destroy test test_fs > # echo $? > 0 > > Now let's get to idempotency itself. Given the response in [1], command is > idempotent only when the output is teh same. Based on the example above it > seems that stratis create is actually not idempotent, as trying to create > the same fs again fails. But I'd like to pinpoint the definition and > application of idempotency first before settling this. I like the > idempotency definition from RFC 7231 [2] (HTTP/1.1), mainly the end of the > paragraph: "repeating the request will have the same intended effect, even > if the original request succeeded, though the response might differ." > Based on this it is actually idempotent to give user different response as > long as the effect is the same, eg. item is present at the end of create > command (unless something crashed). So even if I run the command again and > the item got already created by previous command, the item still present. > The user gets different response, but the state of the server is the same, > hence idempotency stands. Same applies to any other command including > 'stratis fs destroy' - idempotency is not broken as long as the fs is not > present at the end of the command. Response to user does not matter to > idempotency. > > So I think based on examples on what others do (dm, LVM...), 'stratis fs > remove' should actually warn user and fail if he tries to remove something > that is not there. It does not break idempotency and makes the CLI much > nicer to user. > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1688262 > [2] https://tools.ietf.org/html/rfc7231#section-4.2.2 > > > _______________________________________________ > stratis-devel mailing list -- stratis-devel(a)lists.fedorahosted.org > To unsubscribe send an email to stratis-devel-leave(a)lists.fedorahosted.org > Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines > List Archives: > https://lists.fedorahosted.org/archives/list/stratis-devel@lists.fedoraho... >