-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
chore(kv): delete deprecated kv service code #19783
Conversation
@@ -133,41 +134,23 @@ func (s *Service) findTaskByID(ctx context.Context, tx Tx, id influxdb.ID) (*inf | |||
t.LatestCompleted = t.CreatedAt | |||
} | |||
|
|||
// Attempt to fill in the owner ID based on the auth. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note for reader:
https://github.com/influxdata/influxdb/blob/master/kv/migration/all/0003_task_owners.go#L15
This should not be nil anymore as a migration was delivered to backfill missing owners a while ago.
@@ -863,12 +815,6 @@ func (s *Service) deleteTask(ctx context.Context, tx Tx, id influxdb.ID) error { | |||
return influxdb.ErrUnexpectedTaskBucketErr(err) | |||
} | |||
|
|||
if err := s.deleteUserResourceMapping(ctx, tx, influxdb.UserResourceMappingFilter{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't create URMs anymore for tasks (like this anyway). So we don't need this.
This may leave some dangling URMs, but they are harmless cruft.
This includes removal of a lot of kv.Service responsibilities. However, it does not finish the re-wiring. It removes documents, telegrafs, notification rules + endpoints, checks, orgs, users, buckets, passwords, urms, labels and authorizations. There are some oustanding pieces that are needed to get kv service compiling (dashboard service urm dependency). Then all the call sites for kv service need updating and the new implementations of telegraf and notification rules + endpoints needed installing (along with any necessary migrations).
00b9c98
to
c32b20f
Compare
Second pass: #19878 |
Closes #18498
This includes removal of a lot of kv.Service responsibilities. However,
it does not finish the re-wiring.
It removes:
There are some outstanding pieces that are needed to get kv service compiling (dashboard service urm dependency). Then all the call sites for kv service need updating and the new implementations of telegraf and notification rules + endpoints needed installing (along with any necessary migrations).