Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ots upgrade of multiple timestamps halted by single incomplete timestamp #71

Open
dhimmel opened this issue Feb 16, 2018 · 7 comments
Open

Comments

@dhimmel
Copy link

dhimmel commented Feb 16, 2018

The Manubot uses OpenTimestamps during continuous deployment to timestamp scholarly manuscripts. The .ots files are saved in directories and can be upgraded at a later date.

To upgrade, I ran the following command:

ots upgrade  v/*/*.ots

The support for upgrading multiple files at once was very convenient. However, the command exits as soon as a single failure occurs. In my case, one of the timestamps had yet to be confirmed, resulting in:

Calendar https://bob.btc.calendar.opentimestamps.org: Pending confirmation in Bitcoin blockchain
Failed! Timestamp not complete

This triggered the program to exit as per:

else:
logging.warning("Failed! Timestamp not complete")
sys.exit(1)

As a result, many timestamps were not upgraded that could have been upgraded. Would it make sense to not exit on failure and to proceed with additional inputs?

@dhimmel
Copy link
Author

dhimmel commented Feb 16, 2018

A workaround is to use xargs, sorting files by their date modified:

ls -t --reverse v/*/*.ots | xargs ots upgrade

@petertodd
Copy link
Member

petertodd commented Feb 28, 2018

I just checked, and the GNU mv command has the following behavior:

$ mv -v a/* b/* c/* t
‘a/a’ -> ‘t/a’
‘b/b’ -> ‘t/b’
mv: cannot move ‘b/b’ to ‘t/b’: Permission denied
‘c/c’ -> ‘t/c’
$ ls t
a  c

In other words, failures don't cause subsequent actions to fail; cp has similar behavior. So IMO this is a good idea, and would match standard semantics.

Anyone else care to weigh in? @RCasatta ?

@RCasatta
Copy link
Member

RCasatta commented Mar 1, 2018

I agree to match standard commands semantics, so exit 0 in the highlighted code, we should probably change also the message to something like "Temporary failure! Timestamp not yet complete"

@dhimmel
Copy link
Author

dhimmel commented Mar 5, 2018

we should probably change also the message to something like "Temporary failure! Timestamp not yet complete"

Agreed. Incomplete timestamps are not really a failure in the way that an invalid timestamp would be. What is the desired exit code for the following grey areas?

  1. 1 successful upgrade, 1 Invalid timestamp, 1 timestamp not complete
  2. 1 already upgraded, 1 Invalid timestamp, 1 timestamp not complete
  3. 1 successful upgrade, 1 timestamp not complete
  4. 1 already upgraded, 1 timestamp not complete

@petertodd
Copy link
Member

I guess we better do whatever cp and mv etc. do for those types of cases.

@petertodd
Copy link
Member

@dhimmel Thinking about this more, I think the multiple timestamp upgrade functionality was a mistake that overly complicates the design; better for the user to use xargs or similar when they have this problem.

@dhimmel
Copy link
Author

dhimmel commented Mar 13, 2018

the multiple timestamp upgrade functionality was a mistake that overly complicates the design

I agree it complicates the design, but I lean towards OTS dealing with that complexity versus the user.

In the Manubot use case, upgrades will always be batched. Batches will likely consist of .ots files that are already upgraded, not-yet-upgraded but complete, and not-yet-upgraded but incomplete. xargs isn't a great solution: it's confusing, difficult to customize, and not cross-platform. It also short-circuits after a single error, so the only reason the workaround succeeds is because the errors are presumably going to be passed through xargs last.

Multi-timestamp functionality in the python OTS CLI is nice because it's cross-platform and will greatly reduce the implementation burden users face to upgrade multiple timestamps.

I don't have a strong opinions on how the OTS API should work. Perhaps there should be an ots --multi-upgrade option or something. Anyways, if multi-timestamp upgrading is removed, I think users would end up creating a multi-timestamp CLI in python... so why not just have it in the official implementation?

Semi-related, what's the API for upgrading a single timestamp from within python (not using the CLI)? Is the Python API stable or should users always interact with the OTS via the CLI?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants