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

Reorganize script logic for easier debugging #13

Merged
merged 1 commit into from
Jun 21, 2023

Conversation

alexander-bauer
Copy link
Contributor

Hey @mirioeggmann, thank you for publishing this project -- I use it in my home environment, and it's saved me a lot of trouble.

I updated today and didn't RTFM, and so didn't notice that I needed to adjust some of the variables I supply. I took a crack at adjusting the script to be a bit more verbose, and to fail at earlier stages if the variables it requires aren't supplied.

I realize this is a substantial reorganization of the whole core logic, but hopefully you find it to your liking.

Thanks again!
Sasha

@alexander-bauer
Copy link
Contributor Author

Some explanatory notes I neglected to include above:

  • I changed existing echos to use 1>&2 to redirect output to stderr as opposed to stdout; this doesn't much matter here, but keeps output from the script of operator interest consistently on stderr, and programmatic output on stdout.
  • I changed [[ -e "${MYVAR+x}" ]] style tests to [[ -v MYVAR ]], to make them somewhat more succinct.
  • I rearranged the auth header logic to test first for API_TOKEN, and then for both AUTH_EMAIL and AUTH_KEY, and fail loudly if both cases fail. I also extracted this to its own section which sets an AUTH_HEADERS array, which is assembled into the final command at the end.
  • I made the IP and PAYLOAD determination/assembly more verbose, echoing each.
  • I de-duplicated the invocation of the final command by placing its arguments into an array, and then executing it once. Hopefully this will reduce maintenance cost if there's ever another API version change.

@mirioeggmann
Copy link
Owner

Hey @alexander-bauer

Glad to hear that it is being used & thanks a lot for the contribution!🚀 I think these are definitely some reasonable changes.
I will merge the PR and create a new release in the coming weeks.

Cheers
Mirio

@mirioeggmann mirioeggmann merged commit 0fe028d into mirioeggmann:master Jun 21, 2023
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

Successfully merging this pull request may close these issues.

2 participants