DATA-3399: Dataset Data Remove Functions #4780
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…taset data remove ids
viambot
added
the
safe to test
viambot
added
safe to test
viambot
added
safe to test
vpandiarajan20
requested review from
stuqdog,
purplenicole730 and
tahiyasalam
purplenicole730
approved these changes
Feb 6, 2025
cli/app.go
Outdated
| &cli.StringSliceFlag{ | ||
| Name: generalFlagTags, | ||
| Usage: "tags filter. " + | ||
| "accepts tagged for all tagged data, untagged for all untagged data, or a list of tags for all data matching any of the tags", |
There was a problem hiding this comment.
I was a little confused by this usage text. If I'm understanding this correctly, then tagged and untagged are string values you can pass into this argument? If so, I would wrap them in single ' quotes.
cli/app.go
Outdated
| Required: true, | ||
| { | ||
| Name: "filter", | ||
| Usage: "removes binary data from the specified filter from dataset", |
There was a problem hiding this comment.
I would word this differently since it sounds like the data is being removed from the filter.
There was a problem hiding this comment.
Maybe "removes binary data associated with a filter from a dataset"? And then for IDs "removes binary data with specified IDs from a dataset"
tahiyasalam
approved these changes
Feb 10, 2025
cli/app.go
Outdated
| Required: true, | ||
| { | ||
| Name: "filter", | ||
| Usage: "removes binary data from the specified filter from dataset", |
There was a problem hiding this comment.
Maybe "removes binary data associated with a filter from a dataset"? And then for IDs "removes binary data with specified IDs from a dataset"
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
tahiyasalam
reviewed
Feb 11, 2025
cli/data.go
Outdated
| @@ -1025,7 +1025,7 @@ type dataRemoveFromDatasetArgs struct { | |||
| FileIDs []string | |||
| } | |||
|
|
|||
| // DataRemoveFromDataset is the corresponding action for 'data dataset remove'. | |||
| // DataRemoveFromDataset is the corresponding action for 'data dataset remove ids'. | |||
There was a problem hiding this comment.
Should this be dataset data remove?
cli/data.go
Outdated
| DatasetID string | ||
| } | ||
|
|
||
| // DataRemoveFromDatasetByFilter is the corresponding action for 'data dataset remove filter'. |
| }, | ||
| }, | ||
| commonFilterFlags...), | ||
| Action: createCommandWithT[dataAddToDatasetByFilterArgs](DataAddToDatasetByFilter), | ||
| }, | ||
| }, | ||
| }, | ||
| //nolint:dupl |
There was a problem hiding this comment.
What exactly is the nolint referring to since the usage text for add / remove is different?
There was a problem hiding this comment.
cli/app.go:1417: 1417-1474 lines are duplicate of `cli/app.go:1475-1532` (dupl)
{
Name: "add",
Usage: "adds binary data either by IDs or filter to dataset",
UsageText: createUsageText("dataset data add", nil, false, true),
HideHelpCommand: true,
Subcommands: []*cli.Command{
{
Name: "ids",
Usage: "adds binary data with file IDs in a single org and location to dataset",
UsageText: createUsageText(
"dataset data add ids", []string{datasetFlagDatasetID, generalFlagOrgID, dataFlagLocationID, dataFlagFileIDs}, false, false,
),
Flags: []cli.Flag{
&cli.StringFlag{
Name: datasetFlagDatasetID,
Usage: "dataset ID to which data will be added",
Required: true,
},
&cli.StringFlag{
Name: generalFlagOrgID,
Usage: "org ID to which data belongs",
Required: true,
},
&cli.StringFlag{
Name: dataFlagLocationID,
Usage: "location ID to which data belongs",
Required: true,
},
&cli.StringSliceFlag{
Name: dataFlagFileIDs,
Usage: "file IDs of data belonging to specified org and location",
Required: true,
},
},
Action: createCommandWithT[dataAddToDatasetByIDsArgs](DataAddToDatasetByIDs),
},
{
Name: "filter",
Usage: "adds binary data associated with a filter to a dataset",
UsageText: createUsageText("dataset data add filter", []string{datasetFlagDatasetID}, true, false),
Flags: append([]cli.Flag{
&cli.StringFlag{
Name: datasetFlagDatasetID,
Usage: "dataset ID to which data will be added",
Required: true,
},
&cli.StringSliceFlag{
Name: generalFlagTags,
Usage: "tags filter. " +
"accepts 'tagged' for all tagged data, 'untagged' for all untagged data, " +
"or a list of tags for all data matching any of the tags",
},
},
commonFilterFlags...),
Action: createCommandWithT[dataAddToDatasetByFilterArgs](DataAddToDatasetByFilter),
},
},
},
cli/app.go:1475: 1475-1532 lines are duplicate of `cli/app.go:1417-1474` (dupl)
{
Name: "remove",
Usage: "removes binary data either by IDs or filter from dataset",
UsageText: createUsageText("dataset data remove", nil, false, true),
HideHelpCommand: true,
Subcommands: []*cli.Command{
{
Name: "ids",
Usage: "removes binary data with file IDs in a single org and location from a dataset",
UsageText: createUsageText(
"dataset data remove ids", []string{datasetFlagDatasetID, generalFlagOrgID, dataFlagLocationID, dataFlagFileIDs}, false, false,
),
Flags: []cli.Flag{
&cli.StringFlag{
Name: datasetFlagDatasetID,
Usage: "dataset ID from which data will be removed",
Required: true,
},
&cli.StringFlag{
Name: generalFlagOrgID,
Usage: "org ID to which data belongs",
Required: true,
},
&cli.StringFlag{
Name: dataFlagLocationID,
Usage: "location ID to which data belongs",
Required: true,
},
&cli.StringSliceFlag{
Name: dataFlagFileIDs,
Usage: "file IDs of data belonging to specified org and location",
Required: true,
},
},
Action: createCommandWithT[dataRemoveFromDatasetArgs](DataRemoveFromDataset),
},
{
Name: "filter",
Usage: "removes binary data associated with a filter from a dataset",
UsageText: createUsageText("dataset data remove filter", []string{datasetFlagDatasetID}, true, false),
Flags: append([]cli.Flag{
&cli.StringFlag{
Name: datasetFlagDatasetID,
Usage: "dataset ID from which data will be removed",
Required: true,
},
&cli.StringSliceFlag{
Name: generalFlagTags,
Usage: "tags filter. " +
"accepts 'tagged' for all tagged data, 'untagged' for all untagged data, " +
"or a list of tags for all data matching any of the tags",
},
},
commonFilterFlags...),
Action: createCommandWithT[dataRemoveFromDatasetByFilterArgs](DataRemoveFromDatasetByFilter),
},
},
},
I think there's some sort of tolerance built in where it recognizes it as duplicate if only a few characters are changed. Not entirely sure though.
viambot
added
safe to test
tahiyasalam
approved these changes
Feb 11, 2025
vijayvuyyuru
pushed a commit
to vijayvuyyuru/rdk
that referenced
this pull request
Feb 11, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
dataset data removetodataset data remove idsdataset data remove filterManually tested:
viam dataset data add ids --dataset-id=67a512205da1bc26ca01cd5a --location-id=nihu6z0k8c --org-id=0fbe951e-d4c6-427f-985f-784b7b85842c --file-ids=rXu0LkivJtEZojTkmO86xdy5Ft3QovKQfYo17a6EsCNhgCPngHGgab9zj6jCpwLK,9kw4DyE5qxYai0c86xzXXyQNnpN2K26FKT3UzM68WdwmhB4zNValb4bSKkvl7b5Vviam dataset data remove filter --dataset-id=67a512205da1bc26ca01cd5a --location-ids=nihu6z0k8c --start=2023-01-01T05:00:00.000Z --end=2024-10-01T04:00:00.000Z --tags=blue_star