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

Database attributes defaulting to None creates lots of bugs #926

Open
nh2 opened this issue Apr 15, 2018 · 1 comment
Open

Database attributes defaulting to None creates lots of bugs #926

nh2 opened this issue Apr 15, 2018 · 1 comment

Comments

@nh2
Copy link
Contributor

nh2 commented Apr 15, 2018

A common idiom in nixops MachineState subclasses is the initialisation of lots of fields to None. For example:

auth_token = nixops.util.attr_property("digitalOcean.authToken", None)

This results in lots of bugs. For example: #627, #490, #530, #925. These are just the ones related to auth token handling I found within tha last 10 minutes; I have found other ones in the past and still regularly get surprised by it.

Usually, code like

def get_auth_token(self):
return os.environ.get('DIGITAL_OCEAN_AUTH_TOKEN', self.auth_token)
breaks loudly when self.auth_token isn't set; avoiding a bug. But because it's defaulted to None, it silently continues and a bug is introduced.

I think we should not set things to None and then rely on everybody not forgetting to call whatever magic state updating functions.

Instead, if possible, we should construct values with their correct values initially.

I don't know what exactly is responsible for this situation / how it came to be this way. I suspect it's the "self.something variables magically update the sqlite DB, much convenient" logic that nixops currently has instead of explicit separation of what is stored state and what isn't, but I haven't convinced myself of that yet.

@ruhatch
Copy link

ruhatch commented Apr 1, 2019

I am running into this issue right now, specifically with the netmask attribute being null on this line. I'm not sure how to resolve that issue and definitely would like the whole backend to be more robust to this kind of error.

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

2 participants