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

Client command list #202

Closed
46 tasks done
ketiltrout opened this issue Oct 31, 2024 · 5 comments
Closed
46 tasks done

Client command list #202

ketiltrout opened this issue Oct 31, 2024 · 5 comments
Assignees
Labels
cli Issues relating to the alpenhorn CLI investigate This issue needs further investigation question ux User experience improvements.
Milestone

Comments

@ketiltrout
Copy link
Member

ketiltrout commented Oct 31, 2024

A proposal for the client command list, to see what we need. Per #122, we will stick to <noun> <verb>:

@ketiltrout ketiltrout added question investigate This issue needs further investigation cli Issues relating to the alpenhorn CLI labels Oct 31, 2024
@ketiltrout ketiltrout self-assigned this Oct 31, 2024
@ketiltrout ketiltrout added the ux User experience improvements. label Oct 31, 2024
@tjrennie
Copy link

tjrennie commented Nov 1, 2024

This looks like a complete list of commands, but I'd like to see a way of seeing the last n transfers that were cancelled or ignored as well as the last n transfers that were run (which i assume is action list) . - You mentioned this was more server metrics but it would be useful regardless

@ketiltrout
Copy link
Member Author

I've added a transfer list command which I think might do what you want, with the right set of filtering options

ketiltrout added a commit that referenced this issue Nov 4, 2024
Here's a start on getting the client working, which I've done to try to
gauge how much work it will entail. This PR updates and re-implements
command in the `group` command group, which manipulates StorageGroups.
(I think this is the easiest command group to do.)

## Test suite
This PR also re-engineers how the client is run in the test-suite.
There's now a `client` fixture that runs the client via
`click.testing.CliRunner` and performs rudimentary checks on the result.
This abstracts a lot of the routine rigamarole of running the CLI via
pytest, meaning the tests themselves can be relatively simple.

This system uses the same kind of persistent in-memory Sqlite database
that I used for the server end-to-end test to make it possible for the
client process and the test suite itself to access the same ephemeral
database.

## Client
Common command options (of which there are many) are being collected in
`client.options` along with common code to process them.

### Commands
(per the list in #202):

* `create` lets you set I/O class and I/O config. There is no vetting of
the I/O config data based on what the I/O class needs (see discussion in
#165). I think it's best to punt that downfield and worry about it in
the future.
* `list` is pretty simple. Shows I/O class but not I/O config, because I
don't think it fits very well in the tabular format, but I could be
convinced otherwise.
* `modify` has the same comments about I/O config vetting as `create`.
It can also fix broken JSON in the database.
* `rename` trivial
* `show` definitely willing to take suggestions on formatting here, as
well as what StorageNode columns should be presented with
`--node-details`. `--node-stats` is for showing counts/sizes of files
(see the `node stats` command in #202) and will be implemented when
`node stats` is implemented.

## Also
Fixed `common.logger.echo` which was broken from the start.
@ketiltrout
Copy link
Member Author

After getting some familiarity working on the client for a while, I've removed the transfer and action "noun" groups.

I think it's better to limit the groups to "concrete" objects: Groups, Node, Acqs, Files (including File Copies).

I've dispersed the functionality of the removed groups to the other ones. Let me know what you think, and if you can see any gap in functionality that I've missed.

ketiltrout added a commit that referenced this issue Nov 14, 2024
This is based on my suggestion to remove the `action` group in #202,
which I think was ill-defined.

Instead:

* Adds an `--actions` flag to `node show` and `group show` to
  show relevant `StorageTransferAction`s affecting the node/group.
* Adds an `--all` flag to `node show` and `group show` to provide a
  way to list everything without having to know what lists what.
* Adds two new commands to manipulate `StorageTransferAction`s:
  * `group auto-sync GROUP NODE` to set/clear the autosync flag
  * `node auto-clean NODE GROUP` to set/clear the autoclean flag

For a given (NODE, GROUP) pair, these two commands modify the same row
in the `StorageTransferAction` table (the pair has to be unique in the
table).  So, there's ambiguity over which command goes with which "noun".
The way I've done it, the command is associated with the object that the
auto-action modifies (auto-sync adds files to GROUP; auto-clean deletes
files from NODE).  There's probably an argument for doing it the other
way; I'm not sure it matters much, so long as we are consistent about
it.

Also: should the command names have a dash in them?  Or would `group
autosync GROUP NODE` etc. be better?
ketiltrout added a commit that referenced this issue Nov 14, 2024
This is based on my suggestion to remove the `action` group in #202,
which I think was ill-defined.

Instead:

* Adds an `--actions` flag to `node show` and `group show` to
  show relevant `StorageTransferAction`s affecting the node/group.
* Adds an `--all` flag to `node show` and `group show` to provide a
  way to list everything without having to know what lists what.
* Adds two new commands to manipulate `StorageTransferAction`s:
  * `group auto-sync GROUP NODE` to set/clear the autosync flag
  * `node auto-clean NODE GROUP` to set/clear the autoclean flag

For a given (NODE, GROUP) pair, these two commands modify the same row
in the `StorageTransferAction` table (the pair has to be unique in the
table).  So, there's ambiguity over which command goes with which "noun".
The way I've done it, the command is associated with the object that the
auto-action modifies (auto-sync adds files to GROUP; auto-clean deletes
files from NODE).  There's probably an argument for doing it the other
way; I'm not sure it matters much, so long as we are consistent about
it.

Also: should the command names have a dash in them?  Or would `group
autosync GROUP NODE` etc. be better?
ketiltrout added a commit that referenced this issue Nov 14, 2024
This is based on my suggestion to remove the `action` group in #202,
which I think was ill-defined.

Instead:

* Adds an `--actions` flag to `node show` and `group show` to show
relevant `StorageTransferAction`s affecting the node/group.
* Adds an `--all` flag to `node show` and `group show` to provide a way
to list everything without having to know what lists what.
* Adds two new commands to manipulate `StorageTransferAction`s:
  * `group auto-sync GROUP NODE` to set/clear the autosync flag
  * `node auto-clean NODE GROUP` to set/clear the autoclean flag

For a given (NODE, GROUP) pair, these two commands modify the same row
in the `StorageTransferAction` table (the pair has to be unique in the
table). So, there's ambiguity over which command goes with which "noun".
The way I've done it, the command is associated with the object that the
auto-action modifies (auto-sync adds files to GROUP; auto-clean deletes
files from NODE). There's probably an argument for doing it the other
way; I'm not sure it matters much, so long as we are consistent about
it.

Also: should the command names have a dash in them? Or would `group
autosync GROUP NODE` etc. be better?
ketiltrout added a commit that referenced this issue Nov 15, 2024
Not much to fix here. The extension tables aren't handled but they can't
be unless we were to create some sort of new extension type which
provides a list of tables to create.

Given that the only use of such an extension would be to pass them to
`create_tables` in this function, it may be more trouble than it's
worth. Probably easier for both alpenhorn and the extension creators to
just tell them to create their own tables.

In any case, it's very low priority.

I had another command in the `db` group listed in #202 called `status`.
Not sure it's going to get implemented. Not sure what it would tell us.
Perhaps more useful would be an integrity-checking `db check` command.
But neither are on the table at the moment.
@ketiltrout
Copy link
Member Author

ketiltrout commented Nov 18, 2024

After developing the on-demand import framework for the daemon (#235) I've been thinking about the need to have the ability to manually modify Archive entries in the database with the CLI. With that in mind, I've added the following commands to the list above to permit that:

ArchiveAcq

  • acq create -- create a new ArchiveAcq record
    acq create NAME --notes=COMMEMNT

ArchiveFile

  • file create -- create a new ArchiveFile record

    • Directly:
      file create NAME ACQ_NAME --md5=<SUM> --size=<SIZE>

    • Via reading a file:
      file create NAME ACQ_NAME --from-file --prefix=PREFIX
      i.e. stats/md5s local file [PREFIX/]ACQ_NAME/NAME to generate MD5 and size (PREFIX is optional, to tell alpenhorn where to find the file)

      Neither of these methods involve using an import-detect plugin, meaning no third-party callback is invoke. See file import if you want to have that functionality

  • file import -- request import of a file on a node (via ArchiveFileImportRequest)
    file import PATH NODE --register-new

  • file modify -- can be used to change MD5 / size
    file modify PATH --md5=<sum> --size=<SIZE>
    file modify NAME ACQ_NAME --from-file --prefix=PREFIX

ArchiveFileCopy

These command also use the "file" command group for simplicity)

  • file state -- set the state of a file (copy) on a node
    file state PATH NODE_NAME --<STATE> --create --ready/-not-ready

    <STATE> is one of:

    • --present: has_file='Y' wants_file='Y'
    • --cleaned: has_file='Y' wants_file='M'
    • --released: has_file='Y' wants_file='N'
    • --suspect: has_file='M' wants_file='Y'
    • --corrupt: has_file='X' wants_file='Y'
    • --missing: has_file='N' wants_file='Y'
    • --removed: has_file='N' wants_file='N'

    This can be used to create new ArchiveFileCopy records (--create-new)

  • file clean -- manipulate ArchiveFileCopy.wants_file for existing records

ketiltrout added a commit that referenced this issue Nov 28, 2024
After "file list", this was less involved that I feared it would be.

I think there's potential to add more ways of searching for files, but
this is good for now.  There are always going to be limits to how much
selection can be done with the CLI.  For very complex queries, users are
going to have to access the ORM directly and make their own queries.

This is the last command I wanted to implement from the command list.
So, I'm going to say this closes #202, unless someone can come up with
an argument to promote one of the proposed commands I suggested, which
otherwise will get left on the back-burner.

After all these command PRs are merged there is still going to be one or
more clean-up passes to do on the CLI before I'm willing to leave it
alone for the time-being.
ketiltrout added a commit that referenced this issue Nov 30, 2024
After "file list", this was less involved that I feared it would be.

I think there's potential to add more ways of searching for files, but
this is good for now. There are always going to be limits to how much
selection can be done with the CLI. For very complex queries, users are
going to have to access the ORM directly and make their own queries.

This is the last command I wanted to implement from the command list in
#202, unless someone can come up with an argument to promote one of the
proposed commands I suggested, which otherwise will get left on the
back-burner.

After all these command PRs are merged there is still going to be one or
more clean-up passes to do on the CLI before I'm willing to leave it
alone for the time-being.

Closes #248
@ketiltrout
Copy link
Member Author

Everything in my list is now implemented.

@ketiltrout ketiltrout added this to the alpenhorn2 milestone Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Issues relating to the alpenhorn CLI investigate This issue needs further investigation question ux User experience improvements.
Projects
None yet
Development

No branches or pull requests

2 participants