HarrisKirk / blue-green-dreams

2 stars 1 forks source link

Fix latest master build #110

Closed chrissound closed 9 months ago

chrissound commented 10 months ago

This should fix the latest build errors.

@HarrisKirk can you please add these envs to the github secrets:

NGINX_LB_ROOT_PASSWORD
SSH_NGINX_LB_PUBLIC_KEY
SSH_NGINX_LB_PRIVATE_KEY_B64

It fixes:

  1. Switch delete would fail if none existed (I'm not sure how this wasn't caught previously)
  2. Adds the new ENVs to the github action
  3. Adds some exceptions if any of the required ENVs are not set, otherwise they propagate through the program and we end up with confusing generic "expected string but is None" type errors.
HarrisKirk commented 10 months ago

I will do those tasks this weekend and merge. I am on the road now. Thanks for being proactive on that issue.

chrissound commented 10 months ago

This should fix #107

HarrisKirk commented 9 months ago

@chrissound - 1) I have added the 3 secrets. Question - why is the exception thrown if there is no switch (if the goal is to delete it anyway, a lack of existence is not exceptional) Perhaps I missed something. 2) I have removed all linode artifacts so that you have a clean space to test this PR. Please test this code (it failed with the exception) Thanks

chrissound commented 9 months ago

An exception for this case should not be thrown. I don't understand why you're getting this exception for this branch? I do however remember facing this exact issue somewhere else but I believe I fixed it.

Actually reading through it all I think I've confused one of us. I did seem to get a successful test though (see https://github.com/chrissound/blue-green-dreams/actions/runs/7119263639) which is a branch that builds upon this 150491c commit to just add the github actions to run on my fork.

But yes I don't think it's necessary to throw an exception, though possibly it was giving me issues when the return from switch_get got used later on - and I think I was trying to halt on this situation as soon as it happened before it gets propagated.

Will try remove this exception being thrown, just a heads up in case it breaks anything else that I was originally trying to fix.