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

Extract Snapshots Collection #53

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

DJensen94
Copy link

@DJensen94 DJensen94 commented May 14, 2024

Add the snapshots collection and its query to the array of collections to extract

πŸ—£ Description

Add the snapshots collection and its query to the array of collections to extract.

πŸ’­ Motivation and context

We need to pull the snapshots data so the Crossfeed application can recreate the VS report for stakeholders.

πŸ§ͺ Testing

Tested query locally

βœ… Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All future TODOs are captured in issues, which are referenced
    in code comments.
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.

βœ… Pre-merge checklist

  • Confirm that there are no issues/concerns from the AE side with deploying this change to Production.

βœ… Post-merge checklist

  • Dev team: Deploy this code to Production.

Add the snapshots collection and its query to the array of collections to extract
@jsf9k jsf9k added the improvement This issue or pull request will add new or improve existing functionality label May 14, 2024
aws_jobs/cyhy-data-extract.py Outdated Show resolved Hide resolved
move snapshots below requests in collections list
Copy link
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

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

Approved, though I have questions about what exactly this means, but I will talk with @climber-girl about them:

We need to pull the snapshots data so the Crossfeed application can recreate the VS report for stakeholders.

Do you only want snapshot data going forward (i.e. after this PR is merged and deployed) or do you also want the ~1.2 million snapshot documents that already exist in the CyHy DB? If the latter, then we will have to coordinate on that outside of this PR.

@DJensen94
Copy link
Author

Approved, though I have questions about what exactly this means, but I will talk with @climber-girl about them:

We need to pull the snapshots data so the Crossfeed application can recreate the VS report for stakeholders.

Do you only want snapshot data going forward (i.e. after this PR is merged and deployed) or do you also want the ~1.2 million snapshot documents that already exist in the CyHy DB? If the latter, then we will have to coordinate on that outside of this PR.

We only want snapshots going forward.

@dav3r
Copy link
Member

dav3r commented May 14, 2024

Do you only want snapshot data going forward (i.e. after this PR is merged and deployed) or do you also want the ~1.2 million snapshot documents that already exist in the CyHy DB? If the latter, then we will have to coordinate on that outside of this PR.

We only want snapshots going forward.

Excellent, thanks for confirming! πŸ‘

@DJensen94
Copy link
Author

@dav3r I am not authorized to merge, is there anything else that is required before this can be merged?

@dav3r
Copy link
Member

dav3r commented May 14, 2024

@dav3r I am not authorized to merge, is there anything else that is required before this can be merged?

I think all that's left is for you to check any remaining checkboxes in the PR description that apply (e.g. the one about reading the CONTRIBUTING doc), then remove any checkboxes or sections (e.g. the Pre-merge checklist) that don't apply.

Note that I put a couple of post-merge tasks in, one for your team and one for my team.

@DJensen94
Copy link
Author

@dav3r All checks complete, and we have reached out to AE so they are aware a new collection is on the way

@jsf9k
Copy link
Member

jsf9k commented May 15, 2024

Is the "All future TODOs are captured in issues, which are referenced in code comments." checkbox needed here or can that just be deleted? I didn't see any TODOs in this PR.

@dav3r
Copy link
Member

dav3r commented May 15, 2024

@dav3r All checks complete, and we have reached out to AE so they are aware a new collection is on the way

@DJensen94 I just got an email from @jessiebeals stating:

Please do not deploy any changes if they will be automatically pushed to the AE with the daily extracts; we will need to coordinate with ME prior to pushing new data into their environment.

We are going to hold off on merging this PR until I get the green light from the AE side, mainly to ensure that this code is not unintentionally deployed before they are ready. I'm adding the blocked label to this PR for that purpose.

@dav3r dav3r added the blocked This issue or pull request is awaiting the outcome of another issue or pull request label May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked This issue or pull request is awaiting the outcome of another issue or pull request improvement This issue or pull request will add new or improve existing functionality
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

None yet

3 participants