Skip to content

Commit

Permalink
chore: disable data store deletion on both CLI and Backend (#3464)
Browse files Browse the repository at this point in the history
* chore: disable data store deletion on both CLI and Backend

* update datastore logic

* update tests

* update tests
  • Loading branch information
danielbdias committed Dec 20, 2023
1 parent 79abaad commit 53c8990
Show file tree
Hide file tree
Showing 11 changed files with 27 additions and 103 deletions.
1 change: 0 additions & 1 deletion .github/workflows/pull-request.yaml
Expand Up @@ -238,7 +238,6 @@ jobs:
- tracetest-jaeger
- tracetest-opensearch
- tracetest-tempo
- tracetest-no-tracing
- tracetest-provisioning-env
- tracetest-signoz
steps:
Expand Down
1 change: 0 additions & 1 deletion cli/cmd/resources.go
Expand Up @@ -224,7 +224,6 @@ var (
return nil
},
}),
resourcemanager.WithDeleteSuccessMessage("DataStore removed. Defaulting back to no-tracing mode"),
resourcemanager.WithResourceType("DataStore"),
),
).
Expand Down
36 changes: 0 additions & 36 deletions examples/tracetest-no-tracing/docker-compose.yml

This file was deleted.

17 changes: 0 additions & 17 deletions examples/tracetest-no-tracing/tests/list-tests.yaml

This file was deleted.

7 changes: 0 additions & 7 deletions examples/tracetest-no-tracing/tracetest-config.yaml

This file was deleted.

1 change: 1 addition & 0 deletions server/app/app.go
Expand Up @@ -490,6 +490,7 @@ func registerDataStoreResource(repository *datastore.Repository, router *mux.Rou
datastore.ResourceNamePlural,
repository,
resourcemanager.WithTracer(tracer),
resourcemanager.DisableDelete(),
)
manager.RegisterRoutes(router)
provisioner.AddResourceProvisioner(manager)
Expand Down
35 changes: 11 additions & 24 deletions server/datastore/datastore_repository.go
Expand Up @@ -27,6 +27,14 @@ func (r *Repository) SetID(dataStore DataStore, id id.ID) DataStore {

const DataStoreSingleID id.ID = "current"

var defaultDataStore = DataStore{
ID: DataStoreSingleID,
Name: "OTLP",
Type: DataStoreTypeOTLP,
Default: true,
Values: DataStoreValues{},
}

const insertQuery = `
INSERT INTO data_stores (
"id",
Expand Down Expand Up @@ -123,27 +131,6 @@ func (r *Repository) Update(ctx context.Context, dataStore DataStore) (DataStore
return dataStore, nil
}

func (r *Repository) Delete(ctx context.Context, id id.ID) error {
tx, err := r.db.BeginTx(ctx, nil)
if err != nil {
return err
}
defer tx.Rollback()

query, params := sqlutil.Tenant(ctx, deleteQuery, id)
_, err = tx.ExecContext(ctx, query, params...)
if err != nil {
return fmt.Errorf("datastore repository sql exec delete: %w", err)
}

err = tx.Commit()
if err != nil {
return fmt.Errorf("commit: %w", err)
}

return nil
}

const getQuery = `
SELECT
"id",
Expand All @@ -170,9 +157,9 @@ func (r *Repository) Get(ctx context.Context, id id.ID) (DataStore, error) {

dataStore, err := r.readRow(row)
if err != nil && errors.Is(err, sql.ErrNoRows) {
return DataStore{
CreatedAt: newCreateAtDateString(),
}, nil // Assumes an empty datastore
dataStore := defaultDataStore
dataStore.CreatedAt = newCreateAtDateString()
return dataStore, nil // Assumes default datastore
}
if err != nil {
return DataStore{}, fmt.Errorf("datastore repository get sql query: %w", err)
Expand Down
2 changes: 1 addition & 1 deletion server/datastore/datastore_repository_test.go
Expand Up @@ -17,7 +17,6 @@ var (
excludedOperations = rmtests.ExcludeOperations(
rmtests.OperationUpdateNotFound,
rmtests.OperationGetNotFound,
rmtests.OperationDeleteNotFound,
rmtests.OperationListSortSuccess,
rmtests.OperationListNoResults,
)
Expand Down Expand Up @@ -45,6 +44,7 @@ func registerManagerFn(router *mux.Router, db *sql.DB) resourcemanager.Manager {
datastore.ResourceNamePlural,
dataStoreRepository,
resourcemanager.WithIDGen(id.GenerateID),
resourcemanager.DisableDelete(),
)
manager.RegisterRoutes(router)

Expand Down
1 change: 1 addition & 0 deletions server/provisioning/provisioning_test.go
Expand Up @@ -192,6 +192,7 @@ func setup(db *sql.DB) provisioningFixture {
datastore.ResourceName,
datastore.ResourceNamePlural,
f.dataStores,
resourcemanager.DisableDelete(),
)

f.provisioner = provisioning.New(provisioning.WithResourceProvisioners(
Expand Down
Expand Up @@ -42,18 +42,8 @@ func TestDeleteDatastore(t *testing.T) {
require.True(dataStore.Spec.Default)

// When I try to delete the datastore
// Then it should delete with success
// Then it should return a error message, showing that we cannot delete a datastore
result = tracetestcli.Exec(t, "delete datastore --id current", tracetestcli.WithCLIConfig(cliConfig))
helpers.RequireExitCodeEqual(t, result, 0)
require.Contains(result.StdOut, "DataStore removed. Defaulting back to no-tracing mode")

// When I try to get a datastore again
// Then it should return an empty datastore
result = tracetestcli.Exec(t, "get datastore --id current", tracetestcli.WithCLIConfig(cliConfig))
// TODO: we haven't defined a valid output to tell to the user that we are on `no-tracing mode`
helpers.RequireExitCodeEqual(t, result, 0)

dataStore = helpers.UnmarshalYAML[types.DataStoreResource](t, result.StdOut)
require.Equal("DataStore", dataStore.Type)
require.False(dataStore.Spec.Default)
helpers.RequireExitCodeEqual(t, result, 1)
require.Contains(result.StdErr, "resource DataStore does not support the action")
}
13 changes: 10 additions & 3 deletions testing/cli-e2etest/testscenarios/datastore/list_datastore_test.go
Expand Up @@ -36,10 +36,17 @@ func TestListDatastore(t *testing.T) {
// And I have my server recently created

// When I try to list datastore on pretty mode and there is no datastore
// Then it should print an empty table
// Then it should list the default datastore
result := tracetestcli.Exec(t, "list datastore --output pretty", tracetestcli.WithCLIConfig(cliConfig))
helpers.RequireExitCodeEqual(t, result, 0)
require.NotContains(result.StdOut, "current")

parsedTable := helpers.UnmarshalTable(t, result.StdOut)
require.Len(parsedTable, 1)

singleLine := parsedTable[0]

require.Equal("current", singleLine["ID"])
require.Equal("OTLP", singleLine["NAME"])
require.Equal("*", singleLine["DEFAULT"])
})

addListDatastorePreReqs(t, env)
Expand Down

0 comments on commit 53c8990

Please sign in to comment.