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
Fix legacy 1.7 build [with potential downsides] #1401
Conversation
There was some incompatibilities that prevented the build from completing, I've patched them to make it work, commenting out a few lines of code (marked in README) - this is likely break GCE and Azure potentially, but I haven't tested. Works on existing (pre 2.0) nixops project out of the box (with DigitalOcean as the cloud of choice).
nix/azure.nix
Outdated
type = with types; loaOf (submodule fileSystemsOptions); | ||
}; | ||
#fileSystems = mkOption { | ||
# type = with types; loaOf (submodule fileSystemsOptions); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
loaOf
is an alias to attrsOf
now, so why not just change the type to attrsOf
proper and be done with it? Doesn't seem necessary to comment it out and break things.
nix/gce.nix
Outdated
type = with types; loaOf (submodule fileSystemsOptions); | ||
}; | ||
#fileSystems = mkOption { | ||
# type = with types; loaOf (submodule fileSystemsOptions); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same.
@cole-h I've changed the
also: any idea what to put instead of |
@@ -93,7 +93,7 @@ def create(self, defn, check, allow_reboot, allow_recreate): | |||
self.creation_token = None | |||
break | |||
if fs["LifeCycleState"] != "creating": | |||
raise Exception("Elastic File System ‘{0}’ is in unexpected state ‘{1}’".format(fs["LifeCycleState"])) | |||
raise Exception("Elastic File System ‘{0}’ is in unexpected state".format(fs["LifeCycleState"])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This just feels wrong to me. Did a little digging and these docs suggest that the first argument could be fs["Name"]
or fs["FileSystemId"]
, or at the very least, you should instead remove the first placeholder and change the second to be {0}
, as the argument passed to format is the state, so the error would make no sense as is now.
@@ -119,7 +119,7 @@ def create(self, defn, check, allow_reboot, allow_recreate): | |||
self.state = self.UP | |||
break | |||
if mt["LifeCycleState"] != "creating": | |||
raise Exception("Elastic File System mount target ‘{0}’ is in unexpected state ‘{1}’".format(mt["LifeCycleState"])) | |||
raise Exception("Elastic File System mount target ‘{0}’ is in unexpected state".format(mt["LifeCycleState"])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, this suggests probably want mt["MountTargetId"]
this time, or again, change the message to e.g. "Elastic File System mount target is in unexpected state `{0}`"
instead.
nixops/backends/azure_vm.py
Outdated
@@ -1095,7 +1095,7 @@ def restore(self, defn, backup_id, devices=[]): | |||
continue | |||
|
|||
self.log("restoring BLOB {0} from snapshot" | |||
.format(media_link, s_id)) | |||
.format(media_link)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say you should maybe should add the format placeholder, not remove the argument here?
There's not a one-size-fits all replacement, NixOS/nixpkgs#66346 has a summary of the options. |
I'm keeping the filesystem options in gce and azure commented out, because the loaOf was not the reason for build failures
As per NixOS/nixpkgs#66346 suggestion I'm going for 'str' since it's a list of arguments and is only further processed by Python
@mystfox thanks for the suggestions, all done [and Travis is now passing <3]. Previously I was just trying to see if it will work ;) Unfortunately I have no idea how to fix the |
Closing this as it's very likely stale and 1.7 is not seeing any development. Feel free to reopen if it's still relevant. |
There was some incompatibilities that prevented the build from
completing, I've patched them to make it work, commenting out a few
lines of code (marked in README) - this is likely break GCE and Azure
potentially, but I haven't tested. Works on existing (pre 2.0) nixops
project out of the box (with DigitalOcean as the cloud of choice).
But I guess that partially working legacy nixops is better than one that cannot be built