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

Key/value backend #624

Closed
wants to merge 50 commits into from
Closed

Key/value backend #624

wants to merge 50 commits into from

Conversation

moretea
Copy link
Contributor

@moretea moretea commented Mar 13, 2017

There are quite some ideas in the issue tracker about how we could improve nixops radically.

I would like to start out with a feasible iterative plan. As far as I can see, only very trivial SQL operations are every executed against the sqlite3 database; the main use case is to use transactions to support rollback in the case of exceptions. This should not be to hard to implement ourselves in memory, in combination with a key/value server that supports locking certain keys.

  • Refactor the statefile and deployment lock into a specific local implementation
  • Introduce different URI schema's for future backends
  • Refactor all the sqlite3 queries into concrete operations implemented in file.Statefile
    • Deployment
    • Resource
    • VirtualBox
    • GCE done, needs to be verified
    • AWS done, needs to be verified
    • Hetzner done, needs to be verified
  • Implement a local json file backend

raise ex

switcher = {
"file": lambda(url): file.StateFile(url.path),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: wrong indentation

@bjornfor
Copy link
Contributor

I'm not a nixops developer, but IMHO this looks nice.

@moretea
Copy link
Contributor Author

moretea commented Mar 29, 2017

Yes, just found that out myself when trying to run the tests one final time; the issue was that I invoked the tests in another way that did not trigger this :(. The commit that I pushed after your comment should fix that, I'm running the vbox tests again at the moment.

@grahamc
Copy link
Member

grahamc commented Mar 29, 2017

Ok I pulled again and got further, however:

$ nixops deploy --include builder-19 
Traceback (most recent call last):
  File "/nix/store/59qbcipcy3hrykqbxdbiwdzpj8vpxrq8-nixops-1.5.1pre0_abcdef/bin/..nixops-wrapped-wrapped", line 9, in <module>
    import nixops.state.file
ImportError: No module named state.file

@moretea
Copy link
Contributor Author

moretea commented Mar 29, 2017

@grahamc that should fix it!

@moretea moretea mentioned this pull request Mar 29, 2017
@grahamc
Copy link
Member

grahamc commented Mar 29, 2017

Not sure:

$ nixops deploy --include builder-19 
Traceback (most recent call last):
  File "/nix/store/xdl2ql51sclf1yr5xi3cab2qgx03mi0s-nixops-1.5.1pre0_abcdef/bin/..nixops-wrapped-wrapped", line 9, in <module>
    import nixops.state.sqlite3_file
ImportError: No module named state.sqlite3_file

python's import system is a gift that keeps on giving :D

@ip1981
Copy link
Contributor

ip1981 commented May 6, 2017

Wish: don't store in state file parameters that can be extracted from nix expressions or not used anyway: e. g. EC2 instance type, keys, toplevel, storeKeysOnMachine, configsPath, etc.

@nh2
Copy link
Contributor

nh2 commented Jun 20, 2017

What's the motiviation behind this motion? Is it to be able to support other backends than sqlite for the purpose of having the state stored e.g. in a networked database?

@moretea
Copy link
Contributor Author

moretea commented Jun 21, 2017 via email

@rbvermaa
Copy link
Member

Thanks, committed in 308ac51

@rbvermaa rbvermaa closed this Jul 24, 2017
@domenkozar
Copy link
Member

@rbvermaa wrong issue?

@bachp
Copy link
Member

bachp commented Sep 20, 2017

I would like to test this for AWS, what's the easiest way to get this version in my setup?

@bachp
Copy link
Member

bachp commented Oct 11, 2017

@moretea @grahamc I got as far that I now also getting the error

Traceback (most recent call last):
  File "/nix/store/czy2gz21dp72i6lfm1k2gx98ss8kl51x-nixops-1.6pre0_abcdef/bin/..nixops-wrapped-wrapped", line 9, in <module>
    import nixops.state.sqlite3_file
ImportError: No module named state.sqlite3_file

Have you been able to solve this?

@mogorman mogorman mentioned this pull request Feb 4, 2018
12 tasks
@mogorman
Copy link

mogorman commented Feb 4, 2018

fixed merge conflicts in my clone of the fork and will continue modifying the source for this patch there.

@andrewchambers
Copy link

Some explanation of the reason/point of this change would be nice. The original post provides no rationale.

@moretea
Copy link
Contributor Author

moretea commented Mar 9, 2018

The point is that you'll be able to store state remotely eventually, instead of a local sqlite3 file.

Being able to have a key/value that stores to JSON will be a first step to be able to store settings in a remote implementation of that

@bachp
Copy link
Member

bachp commented Jun 13, 2018

@moretea are you still working on this one?

@comozo
Copy link

comozo commented Feb 15, 2019

Hi,
@moretea this feature would be great for us. Is that possible to merge it ?
Ta.

@domenkozar
Copy link
Member

See #1264

@grahamc
Copy link
Member

grahamc commented Mar 26, 2020

Hello!

Thank you for this PR.

In the past several months, some major changes have taken place in
NixOps:

  1. Backends have been removed, preferring a plugin-based architecture.
    Here are some of them:

  2. NixOps Core has been updated to be Python 3 only, and at the
    same time, MyPy type hints have been added and are now strictly
    required during CI.

This is all accumulating in to what I hope will be a NixOps 2.0
release
. There is a tracking issue for that:
#1242 . It is possible that
more core changes will be made to NixOps for this release, with a
focus on simplifying NixOps core and making it easier to use and work
on.

My hope is that by adding types and more thorough automated testing,
it will be easier for contributors to make improvements, and for
contributions like this one to merge in the future.

However, because of the major changes, it has become likely that this
PR cannot merge right now as it is. The backlog of now-unmergable PRs
makes it hard to see which ones are being kept up to date.

If you would like to see this merge, please bring it up to date with
master and reopen it
. If the or mypy type checking fails, please
correct any issues and then reopen it. I will be looking primarily at
open PRs whose tests are all green.

Thank you again for the work you've done here, I am sorry to be
closing it now.

Graham

@grahamc grahamc closed this Mar 26, 2020
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.

None yet