Skip to content
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

Change: Check trash target references before accessing #2121

Closed
wants to merge 7 commits into from

Conversation

mattmundell
Copy link
Contributor

@mattmundell mattmundell commented Jan 2, 2024

What

In the trash case of GET_TARGETS check that dependent resources like credentials exist before trying to access info about them.

Note that this only affects the trash case, because the regular case is already checking resources (eg ssh_credential) before trying to access info (eg credential_name, credential_uuid).

Why

The idea was to speed up the GSA trashcan. The speed seems the same, but we might as well skip the extra queries.

With 1600 trash targets:

time o m m '<get_targets filter="rows=-1 sort=name" trash="1"/>' > /tmp/targ
main: 6.2s, 6.4s, 8.9s
  pr: 6.2s, 8.2s, 7,9s

In the trash case of GET_TARGETS check that dependent resources like
credentials exist before trying to access info about them.  This
speeds up the GSA trashcan, by skipping some SQL queries that would
always return NULL.
Copy link

github-actions bot commented Jan 2, 2024

Conventional Commits Report

Type Number
Changed 1

🚀 Conventional commits found.

@mattmundell mattmundell changed the title Faster trash targets Change: Check trash target references before accessing Jan 2, 2024
@mattmundell mattmundell marked this pull request as ready for review March 7, 2024 14:45
@mattmundell mattmundell requested a review from a team as a code owner March 7, 2024 14:45
@mattmundell mattmundell marked this pull request as draft April 19, 2024 12:00
@mattmundell mattmundell closed this May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant