Don't show deleted objects by default#17
Conversation
|
Note that this is based on #15, so focus on the last commit. Also this will not work till filter support in the server side is merged: osac-project/fulfillment-service#34 . |
jhernand
force-pushed
the
dont_show_deleted_objects_by_default
branch
from
5d67353 to
41016c5
Compare
The more relevant changes in the new versionof the API are the following: - Add the `node_sets` field to clusters. - Add the `HostClass` type and service. Signed-off-by: Juan Hernandez <[email protected]>
This patch adds a new `edit` command that can be used to edit objects of any of the supported types. The command gets the current representation of the object, writes it to a temporary file, invokes an editor to edit it and then, when the user writes the result and closes the editor, it tries to save the result. A good part of the code required for this new command is also required for the `get` command. To avoid duplicating that this patch also moves that to a reusable _ReflectionHelper_ type. Signed-off-by: Juan Hernandez <[email protected]>
This patch adds a new `--filter` flag to the `get` command. The filter
is a CEL expression that the server will use to decide which results to
send.
The object will be returned only if the result of the expression is
`true`.
The `this` variable in this CEL expression refers to the object. Fields
of the object can be used with this as a prefix.
For example, to find all the cluster templates that have a title
starting with `Open`:
```shell
$ ./fulfillment-cli get clustertemplates --filter "this.title.startsWith('Open')"
ID TITLE DESCRIPTION
ocp_4_17_small OpenShift 4.17 small OpenShift 4.17 with `small` instances as worker nodes.
```
Many of the CEL operators and built-in functions are available, but not
100% of them. Documentation about that will be added to the server in
the future.
Signed-off-by: Juan Hernandez <[email protected]>
This patch changes the `get` command so that by default it doesn't show deleted objects. It also adds a new `--include-deleted` flag that changes that behaviour. Signed-off-by: Juan Hernandez <[email protected]>
jhernand
force-pushed
the
dont_show_deleted_objects_by_default
branch
from
41016c5 to
e5b28b0
Compare
| err := finder() | ||
| if err != nil { | ||
| return err | ||
| if !c.includeDeleted { |
There was a problem hiding this comment.
If this filter is done at the CLI level, would this mean that a GUI would have to implement the same filter if it wants to exclude deleted items? Or is the default behavior at the API level to exclude deleted items?
There was a problem hiding this comment.
It is done at the CLI level. So yes, the GUI will have to implement the same filter. Note that it can use this exact logic: it only needs to send filter=!has(this.metadata.deletion_timestamp) with its requests.
There was a problem hiding this comment.
I do think this is a little awkward; it feels like the default behavior should be to ignore deleted objects unless specifically requested; otherwise, all UIs will have to apply the same filter. But I won't object if others disagree!
There was a problem hiding this comment.
I think we need to keep the deleted objects visible to users because there are lengthy delete operators which they may want to track. For example, I expect that when a cluster is deleted it will take time to actually delete it, and it may even eventually fail. In those situations the way we have to let the user know what is happening is looking at the state of the object.
The limitation right now is that we don't have a mechanism for the server to say "this is completely deleted now". If we had it then we could actually remove the deleted records from the database (move them to an "archive" table) and then this wouldn't be a problem.
My suggestion is to use this filter as temporary solution, till we have the complete solution.
2 participants
This patch changes the
getcommand so that by default it doesn't show deleted objects. It also adds a new--include-deletedflag that changes that behaviour.