Turn fleet AGOL feature service updater into a Forklift Pallet #1
Conversation
| raise FileNotFoundError( | ||
| f'known_hosts file {secrets.KNOWNHOSTS} not found. Please ' | ||
| 'create with ssh-keyscan.') |
There was a problem hiding this comment.
thought: I think raising is ok. @stdavis would the other option be to set the pallet status and return from the method? We'd have to see what looks nicer in the logs I suppose.
It does seem like you logged all the other errors so I would continue to be consistent and log this error also before throwing.
There was a problem hiding this comment.
I do think that raising is the correct thing to do and I don't think that logging it is necessary since forklift will log it at a higher level. However, it might be good to test this.
There was a problem hiding this comment.
Ok, I've changed the excepts to do a log.exception("some custom error message") to capture the custom message and then re-raise the original exception. This seemed to be the least verbose way after a little bit of trial and error.
There was a problem hiding this comment.
Let's keep a note to test that we aren't double logging
| raise FileNotFoundError( | ||
| f'known_hosts file {secrets.KNOWNHOSTS} not found. Please ' | ||
| 'create with ssh-keyscan.') |
There was a problem hiding this comment.
I do think that raising is the correct thing to do and I don't think that logging it is necessary since forklift will log it at a higher level. However, it might be good to test this.
jacobdadams
changed the title
jacobdadams
changed the title
Ok, I think I've got this properly palletized. I've extended
Pallet, implemented logging, and created the stand-aloneif __name__ == '__main__'code.The logging tutorials all use %s-style string formatting, but so far fstrings are working just fine. Does that have an impact when running within the full forklift environment?
I'd appreciate a review of how I'm handling exceptions—I'm not sure if I should be logging the messages as well as re-raising the original exception?
I've also made it bomb out if the latest file is old. This is supposed to be a trigger for us that we're not seeing any new data from Fleet. I'm doing so via
sys.exit(message), and if I understand it correctly, including any non-0number will cause it to exit to the system with a code indicating failure/error (ie, not a successful execution). Is this the right way to have this behavior in forklift?