lens: Add ability to view raw data in metabase
#1246
No reviewers
Labels
No labels
P0
P1
P2
P3
badger
frostfs-adm
frostfs-cli
frostfs-ir
frostfs-lens
frostfs-node
good first issue
triage
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
No milestone
No project
No assignees
5 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/frostfs-node#1246
Loading…
Reference in a new issue
No description provided.
Delete branch "a-savchuk/frostfs-node:feat/1223-lens-meta-tui"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Close #1223
Signed-off-by: Aleksey Savchuk a.savchuk@yadro.com
00c79c366f
to474639dba2
77112c4ef8
to0694451e87
a96c2f4e69
to23d1a1ed1b
@ -0,0 +31,4 @@
common.ExitOnErr(cmd, err)
}
func openBoltDB(cmd *cobra.Command) *bbolt.DB {
The command could be run on production databases. It would be nice to have an additional level of security.
I suggest always opening the DB in read-only mode. If we need, we will add
--read-write
flag later.I added an option to
openBoltDB
function and made a database open in read-only mode only for now.@ -0,0 +108,4 @@
return db
}
func isEmptyBucket(b *bbolt.Bucket) bool {
It seems, the name of the function doesn't reflect its purpose.
It checks whether a bucket doesn't contain other buckets?
Removed that function as redundant.
@ -0,0 +158,4 @@
}
func getSubbuckets(bp *bucketProxy) []*bucketProxy {
buckets := make([]*bucketProxy, 0)
2 argument
make
is used when you want to make an informed decision, otherwise default value is ok (var buckets []*bucketProxy
)Removed that function as redundant.
@ -0,0 +74,4 @@
root.
SetSelectable(false).
SetExpanded(true)
cobra.Command
should be a part ofApplication
, as it has.Context()
method.This will allow you to cancel context with Ctrl+C.
nil
, unless you have specific reasons to use[][]byte{}
.Fixed.
@ -0,0 +148,4 @@
SetReference(BucketReference{bucket, next})
node.AddChild(child)
}
}
Does it make sense to have this in defer in the beginning of this function?
Specifically, what happens if we exit with an error on line 217?
Fixed.
@ -0,0 +8,4 @@
"go.etcd.io/bbolt"
)
const (
The variable should be a default value (
defaultBufferSize
) and the size of the buffer should be a parameter of your function.It is important, because it directly affects perfomance and responsiveness.
Fixed.
@ -0,0 +50,4 @@
transform func(key, value []byte) any,
) (<-chan any, error) {
buffer := make(chan any, bufferSize)
It is better to use
db.View
, unless you have specific reasons to manage tx lifecycle by hand. There are lot's of things that could go wrong.Fixed it.
@ -0,0 +55,4 @@
return f
}
func getItemsRecursively(f *tview.Flex) []tview.Primitive {
The default values for slice should be
nil
, likevar items []tview.Primitive
.That component is redundant now. At first, I though I'll have UI with main window split, one view on the left and another on the right. The component was created to make focus change between two views possible.
Because of possible small terminal size (80 symbols width and 25 high) and a lot of data to show I've decided each view to be displayed on a separate page.
I removed that component.
@ -0,0 +61,4 @@
count := f.GetItemCount()
for i := 0; i < count; i++ {
switch item := f.GetItem(i).(type) {
case *tview.Flex:
Depending on how much items to you have, this will scale badly (because you allocate a slice, which you then append and GC).
A better API would be providing the slice to which you append as a parameter to
getItemsRecursively
(nil
by default).Removed that component.
@ -0,0 +29,4 @@
func NewMultipageView() *MultipageView {
view := &MultipageView{
Box: tview.NewBox(),
The default value should be nil (omit the field).
Omit all the fields except
Box
. That is a common approach intview
library:Box
component has a rich interface that's inherited by all other components, that interface facilitates set of custom input handlers, draw of a component's frame and title, and etc.Here's an example from
tview
.@ -0,0 +32,4 @@
items: make([]*MultipageItem, 0),
nextItemFunc: func() *MultipageItem { return nil },
It is ok to omit default values.
Fixed it.
@ -0,0 +52,4 @@
func (v *MultipageView) GetNextItemFunc() func() *MultipageItem {
return v.nextItemFunc
}
Why the getter is called
GetNextItemFunc()
, but the setter isSetPopulateFunc()
?Fixed it.
@ -0,0 +19,4 @@
return func(key, value []byte) (nextKey, nextValue []byte, view *View) {
acc := &View{}
for _, formatter := range formatters {
key, value, view = formatter(key, value)
Not important here, as the items are low in size, but it is usually good to use
strings.Builder
for string construction, as it doesn't allocate intermediate slices.You can then construct
acc
right before you return.Another option is to revise the whole API, formatter could know about the place is writes to (like accepting
io.Writer
).Fixed.
@ -0,0 +29,4 @@
var acc error
for _, h := range hs {
view, next, err = h(key, value)
if err == nil {
return view, next, nil
?Fixed.
@ -0,0 +41,4 @@
// TODO: change the message
// TODO: make predefined error ???
return nil, nil, errors.New("has no prefix")
}
Isn't it
bytes.HasPrefix
from the stdlib?Fixed.
@ -0,0 +79,4 @@
return key, value[32:], nil
}
func TerminationValidator(key, value []byte) (nextKey, nextValue []byte, err error) {
Should be a global error, similar to others.
Also, I do not understand the message. How about sth with
extra data
orexpected EOF, got ...
?Fixed.
@ -51,7 +51,6 @@ func (np *Processor) processNewEpoch(ev netmapEvent.NewEpoch) bool {
if epoch > 0 && np.alphabetState.IsAlphabet() { // estimates are invalid in genesis epoch
err = np.containerWrp.StartEstimation(prm)
Please, rebase and remove it from this PR. If it is still in master, make a PR.
Done, I created a PR #1265.
7c6e43e89e
to04e01ebd4c
18fd55aaf7
toc4e3f5e894
a7f16b4639
to85e6f48b4e
85e6f48b4e
to11f1e0c4f1
11f1e0c4f1
to5b67b98ed2
f664d6cac6
to1a3ce7fc11
c98aba178c
to973e6fdd4f
5355b69bdf
toe7bb68d1cf
cc49dfcb97
to28fce53fc8
eaf379deeb
toda3b22f32f
da3b22f32f
to16a9c7458e
f687a54666
to642af83a81
46f657797f
to71c1adcacc
71c1adcacc
to381f882db3
1e2bd4f666
to4e4b3d84c1
4e4b3d84c1
to356c690c49
356c690c49
to2b2201f60c
2b2201f60c
toffa7bf44ec
@ -0,0 +46,4 @@
}
func runTUI(cmd *cobra.Command) error {
// tview.Styles.PrimaryTextColor = tcell.ColorBlack
Please remove commented code.
Removed.
ffa7bf44ec
tob737910813
b737910813
to16d52d0085
16d52d0085
to7305e8cba1
7305e8cba1
to55c631d8ca
55c631d8ca
toba2df86612
ba2df86612
toc9044fd1c0
c9044fd1c0
to08af2adbb4
08af2adbb4
to9782d2479f
9782d2479f
to7317c43d5f
7317c43d5f
tobac3ed0bcd
bac3ed0bcd
to408ab046b9
408ab046b9
tof45e852b8f
f45e852b8f
to4f941a8492
4f941a8492
to1af2e6cf11
1af2e6cf11
to57a36ff0e5
57a36ff0e5
to65d6e1fa2b
65d6e1fa2b
to15da026da5
15da026da5
toc19caebef1
WIP: lens: Add ability to view raw data in `metabase`to lens: Add ability to view raw data in `metabase`@ -0,0 +13,4 @@
)
var tuiCMD = &cobra.Command{
Use: "tui",
TUI
is a tool name. For command it is better something likemetabase explore
IMHO.Changed the command name.
c19caebef1
toca2ba8e22c
ca2ba8e22c
tob67ddea4da
@ -0,0 +161,4 @@
var locked []oid.ID
var i uint
for ; i < count; i++ {
For me, when a variable is assigned out of
for
-statement, I expect that it's used somewhere else :) How about this?That's a good point. Changed it everywhere.
@ -0,0 +38,4 @@
exitOnZero("must have owners", *numOwners)
exitOnZero("path to metabase not specified", *path)
if *numAttributesPerObj > *numAttributes {
If you introduce
exitIfFalse
(insteadexitOnZero
), then you also can use this one-line manner to exit the programm on error :)WDYT?
I think it's awesome. I've changed it.
@ -0,0 +39,4 @@
}
func tuiFunc(cmd *cobra.Command, _ []string) {
common.ExitOnErr(cmd, runTUI(cmd))
You pass
cmd
torunTUI
. So, you can declarerunTUI
without returningerror
.You have already exit with
common.ExitOnErr(cmd, err)
whenopenDB
is failed. Let's make it for all errorsThat makes sense especially your comment about error handling after opening a database. However, I have several
defer
s need to be called, so I think it'd be easier to handle them inrunTUI
and then exit with an error intuiFunc
. Please feel free to correct me if I'm wrong.I've changed error handling after opening a database, it now returns an error to the calling function.
@ -0,0 +13,4 @@
)
var tuiCMD = &cobra.Command{
Use: "tui",
What does
TUI
stand for? Can you leave a comment, pleaseI've changed the command name based on that comment. Now it looks like this
Also I've made the command help message more clear.
b67ddea4da
toda99d57e91
da99d57e91
toe7af9beb11
e7af9beb11
to3570b7ea19
3570b7ea19
to6a8df845c1
6a8df845c1
to37a903d27a
This is one of the most useful and in-demand tool. Nice!