Fix: Skip s3 upload if record's last-modified time matches DB value #921
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 5f85a6d.
github-actions
bot
added
bug
| @@ -234,7 +234,7 @@ func processRecords(ctx context.Context, s3svc *s3.Client, ddbsvc DynamoDBGetIte | |||
| // is compared with the last-modified date the record on-hand. | |||
| // An upload is initiated when the record on-hand has a last-modified date that is more recent | |||
| // than that of the extant item, or when no extant item exists. | |||
| func processRecord(ctx context.Context, s3svc S3ReadWriteObjectAPI, ddbsvc DynamoDBGetItemAPI, record grantRecord) error { | |||
| func processRecord(ctx context.Context, s3svc S3PutObjectAPI, ddbsvc DynamoDBGetItemAPI, record grantRecord) error { | |||
There was a problem hiding this comment.
This is a simplified interface since we don't need to perform read operations anymore.
Comment on lines
-56
to
+55
| targetGrantId, exists := getItemKey["grant_id"] | ||
| var rvErr error | ||
| if exists { | ||
| if targetGrantId, exists := getItemKey["grant_id"]; exists { |
There was a problem hiding this comment.
This is just a change in variable grouping that I noticed as a readability issue; it has nothing to do with the rest of the PR.
Comment on lines
-605
to
+613
| c := mockS3ReadwriteObjectAPI{ | ||
| mockHeadObjectAPI( | ||
| func(ctx context.Context, params *s3.HeadObjectInput, optFns ...func(*s3.Options)) (*s3.HeadObjectOutput, error) { | ||
| t.Helper() | ||
| return &s3.HeadObjectOutput{}, fmt.Errorf("server error") | ||
| }, | ||
| ), | ||
| mockGetObjectAPI(nil), | ||
| mockPutObjectAPI(nil), | ||
| } | ||
| s3client := mockPutObjectAPI(func(context.Context, *s3.PutObjectInput, ...func(*s3.Options)) (*s3.PutObjectOutput, error) { | ||
| t.Helper() | ||
| require.Fail(t, "PutObject called unexpectedly") | ||
| return nil, nil | ||
| }) | ||
| ddbLookups := make(mockDDBClientGetItemCollection, 0) | ||
| ddbLookups = append(ddbLookups, mockDDBClientGetItemReturnValue{ | ||
| GrantId: string(testOpportunity.OpportunityID), | ||
| ItemLastModified: string(testOpportunity.LastUpdatedDate), | ||
| GetItemErr: errors.New("Some issue with DynamoDB"), | ||
| }) | ||
| err := processRecord(context.TODO(), c, ddbLookups.NewGetItemClient(t), testOpportunity) | ||
| err := processRecord(context.TODO(), s3client, ddbLookups.NewGetItemClient(t), testOpportunity) |
There was a problem hiding this comment.
This change corresponds to the simplified PutObjectAPI argument in the processRecord() signature.
Comment on lines
-627
to
+622
| s3Client := mockS3ReadwriteObjectAPI{ | ||
| mockHeadObjectAPI( | ||
| func(context.Context, *s3.HeadObjectInput, ...func(*s3.Options)) (*s3.HeadObjectOutput, error) { | ||
| t.Helper() | ||
| return nil, &awsTransport.ResponseError{ | ||
| ResponseError: &smithyhttp.ResponseError{Response: &smithyhttp.Response{ | ||
| Response: &http.Response{StatusCode: 404}, | ||
| }}, | ||
| } | ||
| }, | ||
| ), | ||
| mockGetObjectAPI(func(context.Context, *s3.GetObjectInput, ...func(*s3.Options)) (*s3.GetObjectOutput, error) { | ||
| t.Helper() | ||
| require.Fail(t, "GetObject called unexpectedly") | ||
| return nil, nil | ||
| }), | ||
| mockPutObjectAPI(func(context.Context, *s3.PutObjectInput, ...func(*s3.Options)) (*s3.PutObjectOutput, error) { | ||
| t.Helper() | ||
| return nil, fmt.Errorf("some PutObject error") | ||
| }), | ||
| } | ||
| fmt.Printf("%T", s3Client) | ||
| s3Client := mockPutObjectAPI(func(context.Context, *s3.PutObjectInput, ...func(*s3.Options)) (*s3.PutObjectOutput, error) { | ||
| t.Helper() | ||
| return nil, fmt.Errorf("some PutObject error") | ||
| }) |
There was a problem hiding this comment.
This change corresponds to the simplified PutObjectAPI argument in the processRecord() signature.
| ddb := mockDDBClientGetItemCollection([]mockDDBClientGetItemReturnValue{ | ||
| {GrantId: string(testOpportunity.OpportunityID), ItemLastModified: string(testOpportunity.LastUpdatedDate)}, | ||
| // Do not provide a matching record, ensuring that processRecord() will attempt to upload |
There was a problem hiding this comment.
Fixes a test that started to fail once the date-time equality check in processRecord() was updated.
Comment on lines
+630
to
+643
| t.Run("matching LastUpdatedDate skips upload to S3", func(t *testing.T) { | ||
| setupLambdaEnvForTesting(t) | ||
| s3Client := mockPutObjectAPI(func(context.Context, *s3.PutObjectInput, ...func(*s3.Options)) (*s3.PutObjectOutput, error) { | ||
| t.Helper() | ||
| require.Fail(t, "PutObject called unexpectedly") | ||
| return nil, fmt.Errorf("PutObject called unexpectedly") | ||
| }) | ||
| ddb := mockDDBClientGetItemCollection([]mockDDBClientGetItemReturnValue{{ | ||
| GrantId: string(testOpportunity.OpportunityID), | ||
| ItemLastModified: string(testOpportunity.LastUpdatedDate), | ||
| }}) | ||
| err := processRecord(context.TODO(), s3Client, ddb.NewGetItemClient(t), testOpportunity) | ||
| assert.NoError(t, err) | ||
| }) |
There was a problem hiding this comment.
Covers the modified date-time equality check in processRecord().
cmd/SplitGrantsGovXMLDB/s3_test.go
Outdated
There was a problem hiding this comment.
Some of the mock interfaces and structs in this file are no longer needed since processRecord() only performs PutObject operations.
Comment on lines
+76
to
+78
| func (f forecast) dynamoDBItemKey() map[string]ddbtypes.AttributeValue { | ||
| return map[string]ddbtypes.AttributeValue{ | ||
| "grant_id": &ddbtypes.AttributeValueMemberS{Value: string(o.OpportunityID)}, | ||
| "grant_id": &ddbtypes.AttributeValueMemberS{Value: string(f.OpportunityID)}, |
There was a problem hiding this comment.
Just fixing a variable name that I noticed; likely due to a copy/paste flub.
terraform/datadog_dashboard.tf
Outdated
There was a problem hiding this comment.
Note that the dashboard still counts metrics named according to the old (pre-#848) opportunities terminology so that metrics emitted before that change are still represented in the dashboard. We simply add the sums of the two corresponding metrics together in the various graphs.
terraform/staging.tfvars
Outdated
There was a problem hiding this comment.
Updates the Datadog metric definitions according to the new terminology.
as1729
approved these changes
Sep 27, 2024
Terraform Summary
OutputValidation OutputPlan OutputTerraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
+ create
~ update in-place
- destroy
-/+ destroy and then create replacement
Terraform will perform the following actions:
# datadog_dashboard.service_dashboard[0] will be updated in-place
~ resource "datadog_dashboard" "service_dashboard" {
id = "4jj-dh7-k8z"
tags = []
# (9 unchanged attributes hidden)
~ widget {
id = 7709602406748964
~ group_definition {
# (3 unchanged attributes hidden)
~ widget {
id = 3501213286311950
~ timeseries_definition {
# (4 unchanged attributes hidden)
~ request {
# (2 unchanged attributes hidden)
~ formula {
~ formula_expression = "records_skipped" -> "opportunities_skipped + records_skipped"
# (1 unchanged attribute hidden)
# (1 unchanged block hidden)
}
~ formula {
~ formula_expression = "records_updated" -> "opportunities_updated + records_updated"
# (1 unchanged attribute hidden)
# (1 unchanged block hidden)
}
~ formula {
~ formula_expression = "records_created" -> "opportunities_created + records_created"
# (1 unchanged attribute hidden)
# (1 unchanged block hidden)
}
~ formula {
~ formula_expression = "records_failed" -> "opportunities_failed + records_failed"
# (1 unchanged attribute hidden)
# (1 unchanged block hidden)
}
~ query {
~ metric_query {
~ name = "records_skipped" -> "opportunities_skipped"
# (2 unchanged attributes hidden)
}
}
~ query {
~ metric_query {
~ name = "records_updated" -> "records_skipped"
~ query = "sum:grants_ingest.SplitGrantsGovXMLDB.opportunity.updated{$env,$service,$version}.as_count()" -> "sum:grants_ingest.SplitGrantsGovXMLDB.record.skipped{$env,$service,$version}.as_count()"
# (1 unchanged attribute hidden)
}
}
~ query {
~ metric_query {
~ name = "records_created" -> "opportunities_updated"
~ query = "sum:grants_ingest.SplitGrantsGovXMLDB.opportunity.created{$env,$service,$version}.as_count()" -> "sum:grants_ingest.SplitGrantsGovXMLDB.opportunity.updated{$env,$service,$version}.as_count()"
# (1 unchanged attribute hidden)
}
}
~ query {
~ metric_query {
~ name = "records_failed" -> "records_updated"
~ query = "sum:grants_ingest.SplitGrantsGovXMLDB.opportunity.failed{$env,$service,$version}.as_count()" -> "sum:grants_ingest.SplitGrantsGovXMLDB.record.updated{$env,$service,$version}.as_count()"
# (1 unchanged attribute hidden)
}
}
+ query {
+ metric_query {
+ data_source = "metrics"
+ name = "opportunities_created"
+ query = "sum:grants_ingest.SplitGrantsGovXMLDB.opportunity.created{$env,$service,$version}.as_count()"
}
}
+ query {
+ metric_query {
+ data_source = "metrics"
+ name = "records_created"
+ query = "sum:grants_ingest.SplitGrantsGovXMLDB.record.created{$env,$service,$version}.as_count()"
}
}
+ query {
+ metric_query {
+ data_source = "metrics"
+ name = "opportunities_failed"
+ query = "sum:grants_ingest.SplitGrantsGovXMLDB.opportunity.failed{$env,$service,$version}.as_count()"
}
}
+ query {
+ metric_query {
+ data_source = "metrics"
+ name = "records_failed"
+ query = "sum:grants_ingest.SplitGrantsGovXMLDB.record.failed{$env,$service,$version}.as_count()"
}
}
}
# (1 unchanged block hidden)
}
# (1 unchanged block hidden)
}
# (2 unchanged blocks hidden)
}
# (1 unchanged block hidden)
}
# (14 unchanged blocks hidden)
}
# datadog_metric_metadata.custom["grants_ingest.SplitGrantsGovXMLDB.opportunity.created"] will be destroyed
# (because key ["grants_ingest.SplitGrantsGovXMLDB.opportunity.created"] is not in for_each map)
- resource "datadog_metric_metadata" "custom" {
- description = "Count of new grant opportunity records created from Grants.gov data during invocation." -> null
- id = "grants_ingest.SplitGrantsGovXMLDB.opportunity.created" -> null
- metric = "grants_ingest.SplitGrantsGovXMLDB.opportunity.created" -> null
- short_name = "New grant opportunities" -> null
- statsd_interval = 0 -> null
- type = "gauge" -> null
- unit = "record" -> null
}
# datadog_metric_metadata.custom["grants_ingest.SplitGrantsGovXMLDB.opportunity.failed"] will be destroyed
# (because key ["grants_ingest.SplitGrantsGovXMLDB.opportunity.failed"] is not in for_each map)
- resource "datadog_metric_metadata" "custom" {
- description = "Count of grant opportunity records from Grants.gov data that failed to process during invocation." -> null
- id = "grants_ingest.SplitGrantsGovXMLDB.opportunity.failed" -> null
- metric = "grants_ingest.SplitGrantsGovXMLDB.opportunity.failed" -> null
- short_name = "Failed grant opportunities" -> null
- statsd_interval = 0 -> null
- type = "gauge" -> null
- unit = "record" -> null
}
# datadog_metric_metadata.custom["grants_ingest.SplitGrantsGovXMLDB.opportunity.skipped"] will be destroyed
# (because key ["grants_ingest.SplitGrantsGovXMLDB.opportunity.skipped"] is not in for_each map)
- resource "datadog_metric_metadata" "custom" {
- description = "Count of unchanged grant opportunity records from Grants.gov data skipped during invocation." -> null
- id = "grants_ingest.SplitGrantsGovXMLDB.opportunity.skipped" -> null
- metric = "grants_ingest.SplitGrantsGovXMLDB.opportunity.skipped" -> null
- short_name = "Skipped grant opportunities" -> null
- statsd_interval = 0 -> null
- type = "gauge" -> null
- unit = "record" -> null
}
# datadog_metric_metadata.custom["grants_ingest.SplitGrantsGovXMLDB.opportunity.updated"] will be destroyed
# (because key ["grants_ingest.SplitGrantsGovXMLDB.opportunity.updated"] is not in for_each map)
- resource "datadog_metric_metadata" "custom" {
- description = "Count of modified grant opportunity records updated from Grants.gov data during invocation." -> null
- id = "grants_ingest.SplitGrantsGovXMLDB.opportunity.updated" -> null
- metric = "grants_ingest.SplitGrantsGovXMLDB.opportunity.updated" -> null
- short_name = "Updated grant opportunities" -> null
- statsd_interval = 0 -> null
- type = "gauge" -> null
- unit = "record" -> null
}
# datadog_metric_metadata.custom["grants_ingest.SplitGrantsGovXMLDB.record.created"] will be created
+ resource "datadog_metric_metadata" "custom" {
+ description = "Count of new grant records created from Grants.gov data during invocation."
+ id = (known after apply)
+ metric = "grants_ingest.SplitGrantsGovXMLDB.record.created"
+ short_name = "New grant records"
+ type = "gauge"
+ unit = "record"
}
# datadog_metric_metadata.custom["grants_ingest.SplitGrantsGovXMLDB.record.failed"] will be created
+ resource "datadog_metric_metadata" "custom" {
+ description = "Count of grant records from Grants.gov data that failed to process during invocation."
+ id = (known after apply)
+ metric = "grants_ingest.SplitGrantsGovXMLDB.record.failed"
+ short_name = "Failed grant records"
+ type = "gauge"
+ unit = "record"
}
# datadog_metric_metadata.custom["grants_ingest.SplitGrantsGovXMLDB.record.updated"] will be created
+ resource "datadog_metric_metadata" "custom" {
+ description = "Count of modified grant records updated from Grants.gov data during invocation."
+ id = (known after apply)
+ metric = "grants_ingest.SplitGrantsGovXMLDB.record.updated"
+ short_name = "Updated grant records"
+ type = "gauge"
+ unit = "record"
}
# module.DownloadFFISSpreadsheet.module.lambda_function.aws_lambda_function.this[0] will be updated in-place
~ resource "aws_lambda_function" "this" {
id = "grants_ingest-DownloadFFISSpreadsheet"
~ qualified_arn = "arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-DownloadFFISSpreadsheet:62" -> (known after apply)
~ qualified_invoke_arn = "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-DownloadFFISSpreadsheet:62/invocations" -> (known after apply)
tags = {}
~ version = "62" -> (known after apply)
# (21 unchanged attributes hidden)
~ environment {
~ variables = {
~ "DD_TAGS" = "git.commit.sha:db256e68053736078b5fa8c9903d3f4b64927c3d,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:downloadffisspreadsheet" -> "git.commit.sha:d22526e5ef21622c34ae85500eb6c81ae27e8947,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:downloadffisspreadsheet"
~ "DD_VERSION" = "db256e68053736078b5fa8c9903d3f4b64927c3d" -> "d22526e5ef21622c34ae85500eb6c81ae27e8947"
# (11 unchanged elements hidden)
}
}
# (3 unchanged blocks hidden)
}
# module.DownloadFFISSpreadsheet.module.lambda_function.aws_lambda_permission.current_version_triggers["SQSQueueNotification"] must be replaced
-/+ resource "aws_lambda_permission" "current_version_triggers" {
~ id = "SQSQueueNotification" -> (known after apply)
~ qualifier = "62" # forces replacement -> (known after apply) # forces replacement
+ statement_id_prefix = (known after apply)
# (4 unchanged attributes hidden)
}
# module.DownloadGrantsGovDB.module.lambda_function.aws_lambda_function.this[0] will be updated in-place
~ resource "aws_lambda_function" "this" {
id = "grants_ingest-DownloadGrantsGovDB"
~ qualified_arn = "arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-DownloadGrantsGovDB:62" -> (known after apply)
~ qualified_invoke_arn = "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-DownloadGrantsGovDB:62/invocations" -> (known after apply)
tags = {}
~ version = "62" -> (known after apply)
# (21 unchanged attributes hidden)
~ environment {
~ variables = {
~ "DD_TAGS" = "git.commit.sha:db256e68053736078b5fa8c9903d3f4b64927c3d,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:downloadgrantsgovdb" -> "git.commit.sha:d22526e5ef21622c34ae85500eb6c81ae27e8947,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:downloadgrantsgovdb"
~ "DD_VERSION" = "db256e68053736078b5fa8c9903d3f4b64927c3d" -> "d22526e5ef21622c34ae85500eb6c81ae27e8947"
# (13 unchanged elements hidden)
}
}
# (3 unchanged blocks hidden)
}
# module.DownloadGrantsGovDB.module.lambda_function.aws_lambda_permission.current_version_triggers["Schedule"] must be replaced
-/+ resource "aws_lambda_permission" "current_version_triggers" {
~ id = "Schedule" -> (known after apply)
~ qualifier = "62" # forces replacement -> (known after apply) # forces replacement
+ statement_id_prefix = (known after apply)
# (5 unchanged attributes hidden)
}
# module.EnqueueFFISDownload.module.lambda_function.aws_lambda_function.this[0] will be updated in-place
~ resource "aws_lambda_function" "this" {
id = "grants_ingest-EnqueueFFISDownload"
~ qualified_arn = "arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-EnqueueFFISDownload:62" -> (known after apply)
~ qualified_invoke_arn = "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-EnqueueFFISDownload:62/invocations" -> (known after apply)
tags = {}
~ version = "62" -> (known after apply)
# (21 unchanged attributes hidden)
~ environment {
~ variables = {
~ "DD_TAGS" = "git.commit.sha:db256e68053736078b5fa8c9903d3f4b64927c3d,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:enqueueffisdownload" -> "git.commit.sha:d22526e5ef21622c34ae85500eb6c81ae27e8947,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:enqueueffisdownload"
~ "DD_VERSION" = "db256e68053736078b5fa8c9903d3f4b64927c3d" -> "d22526e5ef21622c34ae85500eb6c81ae27e8947"
# (11 unchanged elements hidden)
}
}
# (3 unchanged blocks hidden)
}
# module.EnqueueFFISDownload.module.lambda_function.aws_lambda_permission.current_version_triggers["S3BucketNotification"] must be replaced
-/+ resource "aws_lambda_permission" "current_version_triggers" {
~ id = "S3BucketNotification" -> (known after apply)
~ qualifier = "62" # forces replacement -> (known after apply) # forces replacement
+ statement_id_prefix = (known after apply)
# (5 unchanged attributes hidden)
}
# module.ExtractGrantsGovDBToXML.module.lambda_function.aws_lambda_function.this[0] will be updated in-place
~ resource "aws_lambda_function" "this" {
id = "grants_ingest-ExtractGrantsGovDBToXML"
~ qualified_arn = "arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-ExtractGrantsGovDBToXML:62" -> (known after apply)
~ qualified_invoke_arn = "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-ExtractGrantsGovDBToXML:62/invocations" -> (known after apply)
tags = {}
~ version = "62" -> (known after apply)
# (21 unchanged attributes hidden)
~ environment {
~ variables = {
~ "DD_TAGS" = "git.commit.sha:db256e68053736078b5fa8c9903d3f4b64927c3d,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:extractgrantsgovdbtoxml" -> "git.commit.sha:d22526e5ef21622c34ae85500eb6c81ae27e8947,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:extractgrantsgovdbtoxml"
~ "DD_VERSION" = "db256e68053736078b5fa8c9903d3f4b64927c3d" -> "d22526e5ef21622c34ae85500eb6c81ae27e8947"
# (11 unchanged elements hidden)
}
}
# (3 unchanged blocks hidden)
}
# module.ExtractGrantsGovDBToXML.module.lambda_function.aws_lambda_permission.current_version_triggers["S3BucketNotification"] must be replaced
-/+ resource "aws_lambda_permission" "current_version_triggers" {
~ id = "S3BucketNotification" -> (known after apply)
~ qualifier = "62" # forces replacement -> (known after apply) # forces replacement
+ statement_id_prefix = (known after apply)
# (5 unchanged attributes hidden)
}
# module.PersistFFISData.module.lambda_function.aws_lambda_function.this[0] will be updated in-place
~ resource "aws_lambda_function" "this" {
id = "grants_ingest-PersistFFISData"
~ qualified_arn = "arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-PersistFFISData:62" -> (known after apply)
~ qualified_invoke_arn = "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-PersistFFISData:62/invocations" -> (known after apply)
tags = {}
~ version = "62" -> (known after apply)
# (21 unchanged attributes hidden)
~ environment {
~ variables = {
~ "DD_TAGS" = "git.commit.sha:db256e68053736078b5fa8c9903d3f4b64927c3d,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:persistffisdata" -> "git.commit.sha:d22526e5ef21622c34ae85500eb6c81ae27e8947,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:persistffisdata"
~ "DD_VERSION" = "db256e68053736078b5fa8c9903d3f4b64927c3d" -> "d22526e5ef21622c34ae85500eb6c81ae27e8947"
# (11 unchanged elements hidden)
}
}
# (3 unchanged blocks hidden)
}
# module.PersistFFISData.module.lambda_function.aws_lambda_permission.current_version_triggers["S3BucketNotification"] must be replaced
-/+ resource "aws_lambda_permission" "current_version_triggers" {
~ id = "S3BucketNotification" -> (known after apply)
~ qualifier = "62" # forces replacement -> (known after apply) # forces replacement
+ statement_id_prefix = (known after apply)
# (5 unchanged attributes hidden)
}
# module.PersistGrantsGovXMLDB.module.lambda_function.aws_lambda_function.this[0] will be updated in-place
~ resource "aws_lambda_function" "this" {
id = "grants_ingest-PersistGrantsGovXMLDB"
~ qualified_arn = "arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-PersistGrantsGovXMLDB:62" -> (known after apply)
~ qualified_invoke_arn = "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-PersistGrantsGovXMLDB:62/invocations" -> (known after apply)
tags = {}
~ version = "62" -> (known after apply)
# (21 unchanged attributes hidden)
~ environment {
~ variables = {
~ "DD_TAGS" = "git.commit.sha:db256e68053736078b5fa8c9903d3f4b64927c3d,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:persistgrantsgovxmldb" -> "git.commit.sha:d22526e5ef21622c34ae85500eb6c81ae27e8947,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:persistgrantsgovxmldb"
~ "DD_VERSION" = "db256e68053736078b5fa8c9903d3f4b64927c3d" -> "d22526e5ef21622c34ae85500eb6c81ae27e8947"
# (11 unchanged elements hidden)
}
}
# (3 unchanged blocks hidden)
}
# module.PersistGrantsGovXMLDB.module.lambda_function.aws_lambda_permission.current_version_triggers["S3BucketNotification"] must be replaced
-/+ resource "aws_lambda_permission" "current_version_triggers" {
~ id = "S3BucketNotification" -> (known after apply)
~ qualifier = "62" # forces replacement -> (known after apply) # forces replacement
+ statement_id_prefix = (known after apply)
# (5 unchanged attributes hidden)
}
# module.PublishGrantEvents.module.lambda_function.aws_lambda_function.this[0] will be updated in-place
~ resource "aws_lambda_function" "this" {
id = "grants_ingest-PublishGrantEvents"
~ qualified_arn = "arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-PublishGrantEvents:63" -> (known after apply)
~ qualified_invoke_arn = "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-PublishGrantEvents:63/invocations" -> (known after apply)
tags = {}
~ version = "63" -> (known after apply)
# (21 unchanged attributes hidden)
~ environment {
~ variables = {
~ "DD_TAGS" = "git.commit.sha:db256e68053736078b5fa8c9903d3f4b64927c3d,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:publishgrantevents" -> "git.commit.sha:d22526e5ef21622c34ae85500eb6c81ae27e8947,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:publishgrantevents"
~ "DD_VERSION" = "db256e68053736078b5fa8c9903d3f4b64927c3d" -> "d22526e5ef21622c34ae85500eb6c81ae27e8947"
# (11 unchanged elements hidden)
}
}
# (3 unchanged blocks hidden)
}
# module.PublishGrantEvents.module.lambda_function.aws_lambda_permission.current_version_triggers["dynamodb"] must be replaced
-/+ resource "aws_lambda_permission" "current_version_triggers" {
~ id = "dynamodb" -> (known after apply)
~ qualifier = "63" # forces replacement -> (known after apply) # forces replacement
+ statement_id_prefix = (known after apply)
# (5 unchanged attributes hidden)
}
# module.ReceiveFFISEmail.module.lambda_function.aws_lambda_function.this[0] will be updated in-place
~ resource "aws_lambda_function" "this" {
id = "grants_ingest-ReceiveFFISEmail"
~ qualified_arn = "arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-ReceiveFFISEmail:61" -> (known after apply)
~ qualified_invoke_arn = "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-ReceiveFFISEmail:61/invocations" -> (known after apply)
tags = {}
~ version = "61" -> (known after apply)
# (21 unchanged attributes hidden)
~ environment {
~ variables = {
~ "DD_TAGS" = "git.commit.sha:db256e68053736078b5fa8c9903d3f4b64927c3d,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:receiveffisemail" -> "git.commit.sha:d22526e5ef21622c34ae85500eb6c81ae27e8947,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:receiveffisemail"
~ "DD_VERSION" = "db256e68053736078b5fa8c9903d3f4b64927c3d" -> "d22526e5ef21622c34ae85500eb6c81ae27e8947"
# (12 unchanged elements hidden)
}
}
# (3 unchanged blocks hidden)
}
# module.ReceiveFFISEmail.module.lambda_function.aws_lambda_permission.current_version_triggers["S3BucketNotification"] must be replaced
-/+ resource "aws_lambda_permission" "current_version_triggers" {
~ id = "S3BucketNotification" -> (known after apply)
~ qualifier = "61" # forces replacement -> (known after apply) # forces replacement
+ statement_id_prefix = (known after apply)
# (5 unchanged attributes hidden)
}
# module.SplitFFISSpreadsheet.module.lambda_function.aws_lambda_function.this[0] will be updated in-place
~ resource "aws_lambda_function" "this" {
id = "grants_ingest-SplitFFISSpreadsheet"
~ qualified_arn = "arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-SplitFFISSpreadsheet:62" -> (known after apply)
~ qualified_invoke_arn = "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-SplitFFISSpreadsheet:62/invocations" -> (known after apply)
tags = {}
~ version = "62" -> (known after apply)
# (21 unchanged attributes hidden)
~ environment {
~ variables = {
~ "DD_TAGS" = "git.commit.sha:db256e68053736078b5fa8c9903d3f4b64927c3d,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:splitffisspreadsheet" -> "git.commit.sha:d22526e5ef21622c34ae85500eb6c81ae27e8947,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:splitffisspreadsheet"
~ "DD_VERSION" = "db256e68053736078b5fa8c9903d3f4b64927c3d" -> "d22526e5ef21622c34ae85500eb6c81ae27e8947"
# (14 unchanged elements hidden)
}
}
# (3 unchanged blocks hidden)
}
# module.SplitFFISSpreadsheet.module.lambda_function.aws_lambda_permission.current_version_triggers["S3BucketNotification"] must be replaced
-/+ resource "aws_lambda_permission" "current_version_triggers" {
~ id = "S3BucketNotification" -> (known after apply)
~ qualifier = "62" # forces replacement -> (known after apply) # forces replacement
+ statement_id_prefix = (known after apply)
# (5 unchanged attributes hidden)
}
# module.SplitGrantsGovXMLDB.module.lambda_artifact.aws_s3_object.lambda_function must be replaced
-/+ resource "aws_s3_object" "lambda_function" {
+ acl = (known after apply)
~ arn = "arn:aws:s3:::grantsingest-lambdaartifacts-357150818708-us-west-2/20d1aaa6d456b437e49813e50d29c590.zip" -> (known after apply)
~ bucket_key_enabled = false -> (known after apply)
+ checksum_crc32 = (known after apply)
+ checksum_crc32c = (known after apply)
+ checksum_sha1 = (known after apply)
+ checksum_sha256 = (known after apply)
~ content_type = "binary/octet-stream" -> (known after apply)
~ etag = "63dce65faf8da4b20cd62671a27422a0-3" -> (known after apply)
~ id = "20d1aaa6d456b437e49813e50d29c590.zip" -> (known after apply)
~ key = "20d1aaa6d456b437e49813e50d29c590.zip" -> "80e96fb0b8aa2259bc48dca65a8a2625.zip" # forces replacement
+ kms_key_id = (known after apply)
- metadata = {} -> null
~ storage_class = "STANDARD" -> (known after apply)
- tags = {} -> null
~ version_id = "ghEXKYtqO18ZekelaqwlYhY5LyyfUAfM" -> (known after apply)
# (5 unchanged attributes hidden)
}
# module.SplitGrantsGovXMLDB.module.lambda_function.aws_lambda_function.this[0] will be updated in-place
~ resource "aws_lambda_function" "this" {
id = "grants_ingest-SplitGrantsGovXMLDB"
~ last_modified = "2024-09-25T22:32:35.000+0000" -> (known after apply)
~ qualified_arn = "arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-SplitGrantsGovXMLDB:62" -> (known after apply)
~ qualified_invoke_arn = "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:357150818708:function:grants_ingest-SplitGrantsGovXMLDB:62/invocations" -> (known after apply)
~ s3_key = "20d1aaa6d456b437e49813e50d29c590.zip" -> "80e96fb0b8aa2259bc48dca65a8a2625.zip"
tags = {}
~ version = "62" -> (known after apply)
# (19 unchanged attributes hidden)
~ environment {
~ variables = {
~ "DD_TAGS" = "git.commit.sha:db256e68053736078b5fa8c9903d3f4b64927c3d,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:splitgrantsgovxmldb" -> "git.commit.sha:d22526e5ef21622c34ae85500eb6c81ae27e8947,git.repository_url:github.com/usdigitalresponse/grants-ingest,handlername:splitgrantsgovxmldb"
~ "DD_VERSION" = "db256e68053736078b5fa8c9903d3f4b64927c3d" -> "d22526e5ef21622c34ae85500eb6c81ae27e8947"
# (17 unchanged elements hidden)
}
}
# (3 unchanged blocks hidden)
}
# module.SplitGrantsGovXMLDB.module.lambda_function.aws_lambda_permission.current_version_triggers["S3BucketNotification"] must be replaced
-/+ resource "aws_lambda_permission" "current_version_triggers" {
~ id = "S3BucketNotification" -> (known after apply)
~ qualifier = "62" # forces replacement -> (known after apply) # forces replacement
+ statement_id_prefix = (known after apply)
# (5 unchanged attributes hidden)
}
Plan: 14 to add, 11 to change, 15 to destroy.Pusher: @TylerHendrickson, Action: |
as1729
approved these changes
Sep 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ticket #878
Description
This PR fixes an edge case bug in
SplitGrantsGovXMLDBexposed by #892, where a localgrantOpportunityrecord will be uploaded to replace the currently-stored record when the local record'sLastUpdatedDatematches that of the remote database record.Prior to #892, it would be extremely rare (if not altogether impossible) for a record's
LastUpdatedDate(derived from date, i.e. always-midnight) to exactly match a corresponding S3 object's last-modified timestamp. Presumably, since S3 storage always occurs after the upstream modifications, the S3 storage would always come later. If anything, defaulting to replacing the object in these circumstances would likely be desirable since it would indicate some kind of inaccuracy in the underlying record and/or a previous execution ofSplitGrantsGovXMLDB(and we generally prefer a potential no-op update in the pipeline to the alternative of missing an important grant update).Now, with #892 merged, it is expected that an up-to-date DB record's
LastUpdatedDatewould be exactly equal to that of the record on-hand. We expect that any circumstances where these values are non-equal would mean that the record on-hand has a more recentLastUpdatedDatevalue, and so will always update in those cases. In order to guard against out-of-order record processing, any case where the record on-hand has aLastUpdatedDatevalue that is earlier than that of its corresponding DB record will not result in an update.In addition to the above, this PR also provides the following changes:
recordterminology (e.g.record.skipped) introduced in Ingest forecasted opportunities #848. Previously, dashboards were still looking for the olderopportunity-named metric (e.g.opportunity.skipped), which resulted in missing data on the dashboard.processRecord()function's call signature has been narrowed from requiring methods providing read-write operations (HeadObject/GetObject/PutObject) to a write-only interface that merely requires aPutObjectimplementation. This is possible given the changes from Fix: 878 permissions errors #892, and it probably should have been done in that PR. This has the added bonus of greatly reducing code complexity in unit tests, since we now only need to mockPutObjectcalls instead of mocking all 3 operations.Testing
task test)max_split_grantsgov_records = 20, run the pipeline again, and observe that an additional 10 records were saved, but that the original 10 were not overwritten.PersistGrantsGovXMLDBLambda function was invoked exactly 20 times (10 on the first pipeline run, and 10 on the second) using CloudWatch metrics in LocalStack."Skipping record upload because the extant record is up-to-date"is logged the expected number (e.g. 10) times.Automated and Unit Tests
Manual tests for Reviewer
Checklist