Remove unsafely_update_root_if_necessary from  #1174

Activity

1. jku commented on Oct 12, 2020

   

   jku

   on Oct 12, 2020

   Last edited by jku

   Member

   More actions

   I think the argument is useful: it just does something different from the name or the documentation.

       except tuf.exceptions.ExpiredMetadataError:
         # Raise 'tuf.exceptions.NoWorkingMirrorError' if a valid (not
         # expired, properly signed, and valid metadata) 'root.json' cannot be
         # installed.
         if unsafely_update_root_if_necessary:
           logger.info('Expired Root metadata was loaded from disk.'
             '  Try to update it now.' )
   
         # The caller explicitly requested not to unsafely fetch an expired Root.
         else:
           logger.info('An expired Root metadata was loaded and must be updated.')
           raise
   

   Even the comment is confused but the code seems reasonable to me: by default (arg is True) continue to update metadata even if local root metadata has expired. But if caller explicitly set arg to False, fail on expired local root metadata.

   This seems like a valid feature to have: some clients may well want to fail on expiry. It's just that the name, the comments and the documentation seem to be wrong.

   Removing the argument or modifying the argument name is a breaking change.

   👍React with 👍1Reacted by Joshua Lock
2. joshuagl

   added

   clientRelated to the client (updater) implementationRelated to the client (updater) implementation

   documentationDocumentation of the project as well as procedural documentationDocumentation of the project as well as procedural documentation

   on Oct 14, 2020
3. joshuagl commented on Oct 14, 2020

   

   joshuagl

   on Oct 14, 2020

   Member

   More actions

   Thank you for taking the time to report this.

   Jussi's understanding matches my own, we need this argument during refresh to update from a potentially expired trusted (or bootstrap) root metadata to the latest root metadata.

   That it is confusing indicated we absolutely should fix the documentation. However, I don't think we should change the name at this time as we can't do so without potentially breaking users.

   We can consider changing the name in a future breaking update such as the proposed refactor.
4. jku commented on Nov 30, 2021

   

   jku

   on Nov 30, 2021

   Member

   More actions

   Closing: we are not changing legacy client and ngclient does not have the same design.
5. jku

   closed this as completedon Nov 30, 2021