[IMP] databases: link a repository to a database from the command line - #107
Conversation
lse-odoo
left a comment
lse-odoo
left a comment
There was a problem hiding this comment.
functionallity speacking, would it make sense to also have a command to get the actual current data ? for example giving:
odev database test repository
would give the current directory while:
odev database test repository "lse-odoo/repository"
would set its value
So more like the |
I feel like it's becoming more and more complicated personally. The more command we add, the more the complexity it gets. The ideas are good in themselves, but I guess it can become complicate for a new comers. I am also wondering if "database" command name should be rather rename "local-database" as here all commands make sense only for database installed locally, it might avoid ambiguity on the scope of the action. In the long term I feel that having "group" of commands might make sense in odev to avoid having 10 000 commands listed in help and have a better separation of the "category" of commands (even if it would be arguable in which category belongs which command). This would make sense with plugin in my opinion as generally they include commands for a given scope/category. Here for example being in the group "local-database" so that any odev command have to be prefixed with "local-database" before calling the real command "info, list, etc." could make sense |
Allow change states and parameters of a local database without running it: - change or remove linked worktree - change or remove linked virtual environment - change or remove linked repository - (un)whitelist
6969557 to
4faa3c4
Compare
Reworks the `odev database` command so that it edits databases through the model rather than writing to the data store directly, and adds the same capability to `odev code <database> <repository>`, which is the invocation issue #101 reports. `odev code` accepting both a database and a repository has worked since odev-plugin-editor-base@abd6288 removed the error the issue was filed for, but the repository applied to that single invocation and was never saved, so the next `odev code <database>` had lost it again. It is now persisted, which is also what makes the database usable with every other command relying on that link. Both go through the new `LocalDatabase.link_repository`, which normalizes its argument through `GitConnector`: repository names, HTTPS and SSH URLs and paths to local clones are all accepted and stored as `organization/repository`. Storing the raw argument, as the command did, corrupted the link for anything but a bare name: the stored value is split on its first slash when read back, so a URL yielded an organization of `https:`. Other fixes to the command: - Values were written with `UPDATE ... WHERE name = ...`, which matches no row for a database that odev has never run, so setting a parameter on one silently did nothing while reporting success. Setting now goes through the model, whose write is an upsert. - Changing the repository left the branch of the previous one attached to the new one. The `repository` setter now clears the cached branch. - `--set-repo` and `--remove-repo` (and their venv and worktree counterparts) could be passed together, in which case the removal silently won. They are now rejected as mutually exclusive. `Command._exclusive_arguments` cannot express this: it requires exactly one argument of the group to be present, making the group mandatory as well as exclusive. - Called with no argument but the database, the command did nothing and said nothing; it now prints the current parameters, as asked in review. - `GitConnector` errors surfaced raw instead of going through `self.error`. `StoreDatabases.set_value` is kept, since clearing a value is the one thing that cannot go through the model — the properties fall back to reading the data store when their cached value is empty, and would write the cleared value straight back. It now binds its value as a query parameter and checks the column against the table definition, instead of interpolating both into the query: `repr()` quotes strings containing a quote with double quotes, which PostgreSQL reads as an identifier. `PostgresDatabase.query` grew the `params` argument it needs to forward for that, which `PostgresConnector.query` already accepted. Two supporting fixes: - `GitCommand.worktrees` read `self.args.version` guarded only by the presence of `args`. A command removing that argument has no `version` attribute at all, so any use of `grouped_worktrees` raised `AttributeError`. - `args.Flag` dropped the default it was given when passed an explicit action, so `args.FlagOptional` could not be given one. Closes #101
`args.FlagOptional` is the argument the `database` command relies on to tell a value being turned off apart from a value being left alone, and the tutorial only documented `args.Flag`. Claude-Session: https://claude.ai/code/session_01K8csZBrrBYp8oqH5paxTAm
brinkflew
changed the title
lse-odoo
left a comment
lse-odoo
left a comment
There was a problem hiding this comment.
didn't tested, but the codes looks perfect to me
2 participants
Description
Adds
odev database, a command editing the parameters of a local database without running it —linked repository, virtual environment, worktree and whitelisting — and gives
odev code <database> <repository>the same ability to persist a repository, which is the invocation issue #101 reports.odev codeaccepting both a database and a repository has worked sinceodev-plugin-editor-base@abd6288 removed the error the issue was filed for. What is still broken is
that the repository applied to that single invocation and was never saved, so the next
odev code <database>had lost it again. Persisting it is also what makes the database usable with everyother command relying on that link. The plugin side is
odoo-odev/odev-plugin-editor-base#8 and needs this PR first.
Reworked since the first review
Both callers go through the new
LocalDatabase.link_repository, which normalizes its argumentthrough
GitConnector: repository names, HTTPS and SSH URLs and paths to local clones are allaccepted and stored as
organization/repository. Storing the raw argument, as the command did,corrupted the link for anything but a bare name — the stored value is split on its first slash when
read back, so a URL yielded an organization of
https:.UPDATE ... WHERE name = ..., which matches no row for a database odevhas never run, so setting a parameter on one silently did nothing while reporting success.
Setting now goes through the model, whose write is an upsert.
repositorysetter now clears the cached branch.--set-repoand--remove-repo(and their venv and worktree counterparts) could be passedtogether, in which case the removal silently won. They are now rejected as mutually exclusive.
Command._exclusive_argumentscannot express this: it requires exactly one argument of the groupto be present, which makes the group mandatory as well as exclusive, and these are all optional.
prints the current parameters — @lse-odoo's review comment, answered without a second command:
odev database <name>reads,odev database <name> --set-…writes.GitConnectorerrors surfaced raw instead of going throughself.error.StoreDatabases.set_valueis kept, because clearing a value is the one thing that cannot go throughthe model: the properties fall back to reading the data store when their cached value is empty, and
would write the cleared value straight back. It now binds its value as a query parameter and checks
the column against the table definition instead of interpolating both into the query —
repr()quotes a string containing a quote with double quotes, which PostgreSQL reads as an identifier.
PostgresDatabase.querygrew theparamsargument it needs to forward for that, whichPostgresConnector.queryalready accepted.Two supporting fixes, both needed by the command and both bugs in their own right:
GitCommand.worktreesreadself.args.versionguarded only by the presence ofargs. A commandremoving that argument has no
versionattribute at all, so any use ofgrouped_worktreesraisedAttributeError.args.Flagdropped the default it was given when passed an explicit action, so the newargs.FlagOptionalcould not be given one.Note for reviewers
args.FlagOptionalis a three-state flag:--whitelistsetsTrue,--no-whitelistsetsFalse,and omitting both leaves the stored value alone. It is documented alongside
args.Flagin thecommands tutorial.
Tests live in
tests/tests/commands/test_database_command.py: a name, an HTTPS URL and an SSH URLall store
organization/repository;--set-repoon a database with no store row creates it (thesilent-no-op guard); set and remove together are rejected and leave the store untouched; the three
whitelist states; relinking clears a stale branch; and an unknown worktree errors, which is the case
that fails without the
GitCommand.worktreesfix. Full test suite passes.Linked Issues
Compliance
docsdirectoryrequirements.txtfile, if any🤖 Generated with Claude Code
https://claude.ai/code/session_01K8csZBrrBYp8oqH5paxTAm