Skip to content

Add ig-request data-pull task#8391

Merged
zachmargolis merged 3 commits intomainfrom
margolis-ig-request-data-pull
May 16, 2023
Merged

Add ig-request data-pull task#8391
zachmargolis merged 3 commits intomainfrom
margolis-ig-request-data-pull

Conversation

@zachmargolis
Copy link
Contributor

🛠 Summary of changes

Takes advantage of our new data-pull infra to allow for pulling data from prod in a batched way.

Once merged, this will let us replace the first two separate tasks in our guide:

  1. (open interactive SSM shell & sudo)
  2. data_requests:lookup_users_by_device
  3. data_requests:create_users_report

with one task we can run right from local devops repo:

./bin/data-pull ig-request UUID1 UUID2 UUID3 --requesting-issuer "a:b:c:d"

and it'll print the JSON right there

changelog: Internal, Reporting, Add data-pull subtask to streamling IG reports
@zachmargolis zachmargolis requested review from a team May 15, 2023 19:18
end

result = subtask_class.new.run(args: argv, include_missing: config.include_missing?)
result = subtask_class.new.run(args: argv, config:)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @olatifflexion this is definitely going to cause merge conflicts with your branch to add a new task, sorry! I think it should be pretty stable now

Copy link
Contributor

@olatifflexion olatifflexion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

class InspectorGeneralRequest
def run(args:, config:)
require 'data_requests/deployed'
ActiveRecord::Base.connection.execute('SET statement_timeout = 0')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this safe because this is a one-off command invocation, thus not using a connection pool where this might affect other queries?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it'll be run in its own process outside of any web requests

'uuid-lookup' => UuidLookup,
'uuid-convert' => UuidConvert,
'email-lookup' => EmailLookup,
'ig-request' => InspectorGeneralRequest,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Petty nit: the existing naming convention seems to be things you can look up about a user/account -- their uuid, their email, but not their Inspector General. Would this make more sense as shared-devices or something? It's actually not abundantly clear to me what this is for.

That said, that may be the point?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inspector general is a GSA office, so we're running the standard report with the attributes that the GSA office asks for. And the final export of this includes more than just their devices so I feel like that name might sell it short

remove comment
@zachmargolis zachmargolis merged commit 06b8b24 into main May 16, 2023
@zachmargolis zachmargolis deleted the margolis-ig-request-data-pull branch May 16, 2023 16:37
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.

4 participants