Upgraded AWS dependencies to 1.0 #33

GurvirDehal · 2023-11-28T23:29:07Z

Description of changes

Upgraded the AWS dependencies in the Cargo.toml to 1.0. As a result some things broke and needed fixing, here are the types of changes done in this PR:

All builders have to be unwrapped following the call to .build().
In order to be successfully unwrapped, all builders must be passed input for the parameters that they require.
- Ex: GetPolicyOutput will fail to build unless passed a PolicyStoreId, PolicyId, CreatedTime etc.
The smithy model for the AVP api responses has been updated to not wrap required parameters with Option
- Previously the field GetPolicyOutput.policy_id was Option<String>, now it is just String.
- Therefore we don't need any checks or tests for the Some or None cases and they have been removed in the PR.
The Unhandled variant in the AVP api exceptions is deprecated and shouldn't be used directly.
- Instead this variant can be created with the method unhandled on the exception instead
The aws client now takes a mandatory parameter BehaviorVersion
- BehaviorVersion
Testing and mocking the aws client is done through the following now:
- StaticReplayClient

Issue #, if available

Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

A change (breaking or otherwise) that only impacts unreleased or experimental code.

I confirm that this PR (choose one, and delete the other options):

Does not update the CHANGELOG because my change does not significantly impact released code.

Testing

Ran the./scripts/build_and_test.sh script and ran the integration tests on my aws account. Everything passed.

Disclaimer

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

github-actions · 2023-11-28T23:47:42Z

Coverage after merging upgrade-dependencies into main

65.28%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
src/private/sources
retry.rs	76.47%	100%	33.33%	100%
mod.rs	81.25%	100%	81.25%	100%
src/private/sources/cache
policy.rs	78.85%	100%	74.36%	92.31%	77
template.rs	78.85%	100%	74.36%	92.31%	78
src/private/sources/policy
loader.rs	67.35%	100%	53.85%	82.61%	37, 53–55
reader.rs	75%	100%	42.86%	97.50%	80
core.rs	32.92%	100%	15.75%	97.06%	78
error.rs	80.56%	100%	70.83%	100%
src/private/sources/schema
error.rs	71.43%	100%	58.82%	100%
core.rs	44.23%	100%	27.50%	100%
reader.rs	54.93%	100%	29.55%	96.30%	56
src/private/sources/template
error.rs	67.14%	100%	61.36%	76.92%	76–78, 82–84
core.rs	37.50%	100%	15.79%	96.43%	66
reader.rs	76.39%	100%	42.86%	97.73%	87
loader.rs	68%	100%	47.83%	85.19%	38, 60–62
src/private/translator
avp_to_cedar.rs	40.48%	100%	39.13%	46.67%	114, 130, 132, 46, 66, 93–95
error.rs	25%	100%	25%	100%
src/private/types
template_id.rs	76%	100%	72.73%	100%
policy_store_id.rs	63.33%	100%	66.67%	50%	19–21
policy_id.rs	63.33%	100%	66.67%	50%	18–20
src/public
entity_provider.rs	0%	100%	0%	0%	103–107, 109–112, 129, 144–147, 152–157, 159–160, 162–169, 171, 176, 178, 182–188, 43–45, 78, 93
policy_set_provider.rs	0%	100%	0%	0%	113, 127–135, 139–147, 152, 161, 165–170, 172–177, 180–185, 201–204, 209–210, 212–219, 223–230, 233–241, 244–252, 254–282, 287–293, 58–60, 95
client.rs	91.38%	100%	69.23%	97.78%	57

src/private/translator/avp_to_cedar.rs

src/private/sources/cache/policy.rs

src/private/sources/policy/loader.rs

src/private/sources/schema/core.rs

src/private/translator/avp_to_cedar.rs

src/private/translator/error.rs

src/private/sources/policy/core.rs

src/private/translator/avp_to_cedar.rs

github-actions · 2023-11-29T19:32:54Z

Coverage after merging upgrade-dependencies into main

65.28%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
src/private/sources
retry.rs	76.47%	100%	33.33%	100%
mod.rs	81.25%	100%	81.25%	100%
src/private/sources/cache
policy.rs	78.85%	100%	74.36%	92.31%	77
template.rs	78.85%	100%	74.36%	92.31%	78
src/private/sources/policy
loader.rs	67.35%	100%	53.85%	82.61%	37, 53–55
reader.rs	75%	100%	42.86%	97.50%	80
core.rs	32.92%	100%	15.75%	97.06%	78
error.rs	80.56%	100%	70.83%	100%
src/private/sources/schema
error.rs	71.43%	100%	58.82%	100%
core.rs	44.23%	100%	27.50%	100%
reader.rs	54.93%	100%	29.55%	96.30%	56
src/private/sources/template
error.rs	67.14%	100%	61.36%	76.92%	76–78, 82–84
core.rs	37.50%	100%	15.79%	96.43%	66
reader.rs	76.39%	100%	42.86%	97.73%	87
loader.rs	68%	100%	47.83%	85.19%	38, 60–62
src/private/translator
avp_to_cedar.rs	40.48%	100%	39.13%	46.67%	114, 130, 132, 46, 66, 93–95
error.rs	25%	100%	25%	100%
src/private/types
template_id.rs	76%	100%	72.73%	100%
policy_store_id.rs	63.33%	100%	66.67%	50%	19–21
policy_id.rs	63.33%	100%	66.67%	50%	18–20
src/public
entity_provider.rs	0%	100%	0%	0%	103–107, 109–112, 129, 144–147, 152–157, 159–160, 162–169, 171, 176, 178, 182–188, 43–45, 78, 93
policy_set_provider.rs	0%	100%	0%	0%	113, 127–135, 139–147, 152, 161, 165–170, 172–177, 180–185, 201–204, 209–210, 212–219, 223–230, 233–241, 244–252, 254–282, 287–293, 58–60, 95
client.rs	91.38%	100%	69.23%	97.78%	57

github-actions · 2023-11-30T00:21:51Z

Coverage after merging upgrade-dependencies into main

65.28%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
src/private/sources
retry.rs	76.47%	100%	33.33%	100%
mod.rs	81.25%	100%	81.25%	100%
src/private/sources/cache
policy.rs	78.85%	100%	74.36%	92.31%	77
template.rs	78.85%	100%	74.36%	92.31%	78
src/private/sources/policy
loader.rs	67.35%	100%	53.85%	82.61%	37, 53–55
reader.rs	75%	100%	42.86%	97.50%	80
core.rs	32.92%	100%	15.75%	97.06%	78
error.rs	80.56%	100%	70.83%	100%
src/private/sources/schema
error.rs	71.43%	100%	58.82%	100%
core.rs	44.23%	100%	27.50%	100%
reader.rs	54.93%	100%	29.55%	96.30%	56
src/private/sources/template
error.rs	67.14%	100%	61.36%	76.92%	76–78, 82–84
core.rs	37.50%	100%	15.79%	96.43%	66
reader.rs	76.39%	100%	42.86%	97.73%	87
loader.rs	68%	100%	47.83%	85.19%	38, 60–62
src/private/translator
avp_to_cedar.rs	40.48%	100%	39.13%	46.67%	114, 130, 132, 46, 66, 93–95
error.rs	25%	100%	25%	100%
src/private/types
template_id.rs	76%	100%	72.73%	100%
policy_store_id.rs	63.33%	100%	66.67%	50%	19–21
policy_id.rs	63.33%	100%	66.67%	50%	18–20
src/public
entity_provider.rs	0%	100%	0%	0%	103–107, 109–112, 129, 144–147, 152–157, 159–160, 162–169, 171, 176, 178, 182–188, 43–45, 78, 93
policy_set_provider.rs	0%	100%	0%	0%	113, 127–135, 139–147, 152, 161, 165–170, 172–177, 180–185, 201–204, 209–210, 212–219, 223–230, 233–241, 244–252, 254–282, 287–293, 58–60, 95
client.rs	91.38%	100%	69.23%	97.78%	57

src/public/client.rs

ericox

Seems pretty straight forward/mechanical. I carefully looked at the changes around unwrap() calls after build() invocations. I didn't see any changes in non-test code. It looks like the reason is because we aren't using the AVP API GetPolicyOutput/GetPolicyInput, etc types directly. We are using the sdk client which hasn't been effected by the upgrade correct?

src/private/sources/cache/policy.rs

src/public/client.rs

GurvirDehal · 2023-11-30T18:42:25Z

Seems pretty straight forward/mechanical. I carefully looked at the changes around unwrap() calls after build() invocations. I didn't see any changes in non-test code. It looks like the reason is because we aren't using the AVP API GetPolicyOutput/GetPolicyInput, etc types directly. We are using the sdk client which hasn't been effected by the upgrade correct?

There are some changes in non-test code, some of them being in the translator. You're right that we aren't building the GetPolicyOutput type and so we don't need to worry about the new builder errors, but the types themselves have changed. Namely before, the policy_id from that struct was type Option<String> whereas now its just String. Most of the non-test related changes are for accommodating that, and removing exceptions related to that which are no longer possible such as policy_id_not_found.

github-actions · 2023-11-30T19:05:19Z

Coverage after merging upgrade-dependencies into main

65.28%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
src/private/sources
retry.rs	76.47%	100%	33.33%	100%
mod.rs	81.25%	100%	81.25%	100%
src/private/sources/cache
policy.rs	79.25%	100%	75%	92.31%	77
template.rs	79.25%	100%	75%	92.31%	78
src/private/sources/policy
loader.rs	67.35%	100%	53.85%	82.61%	37, 53–55
reader.rs	75%	100%	42.86%	97.50%	80
core.rs	32.92%	100%	15.75%	97.06%	78
error.rs	80.56%	100%	70.83%	100%
src/private/sources/schema
error.rs	71.43%	100%	58.82%	100%
core.rs	44.23%	100%	27.50%	100%
reader.rs	54.93%	100%	29.55%	96.30%	56
src/private/sources/template
error.rs	67.14%	100%	61.36%	76.92%	76–78, 82–84
core.rs	37.50%	100%	15.79%	96.43%	66
reader.rs	76.39%	100%	42.86%	97.73%	87
loader.rs	68%	100%	47.83%	85.19%	38, 60–62
src/private/translator
avp_to_cedar.rs	40.48%	100%	39.13%	46.67%	114, 130, 132, 46, 66, 93–95
error.rs	25%	100%	25%	100%
src/private/types
template_id.rs	76%	100%	72.73%	100%
policy_store_id.rs	63.33%	100%	66.67%	50%	19–21
policy_id.rs	63.33%	100%	66.67%	50%	18–20
src/public
entity_provider.rs	0%	100%	0%	0%	103–107, 109–112, 129, 144–147, 152–157, 159–160, 162–169, 171, 176, 178, 182–188, 43–45, 78, 93
policy_set_provider.rs	0%	100%	0%	0%	113, 127–135, 139–147, 152, 161, 165–170, 172–177, 180–185, 201–204, 209–210, 212–219, 223–230, 233–241, 244–252, 254–282, 287–293, 58–60, 95
client.rs	91.38%	100%	69.23%	97.78%	57

camarcio

Looks good to me!

Upgraded AWS dependencies to 1.0

1386dd9

GurvirDehal requested review from camarcio, ericox, mtrottie, jiaqiwl, qingsart, maxzcxu, ShiromMakkad and alannulli November 28, 2023 23:50

jiaqiwl reviewed Nov 29, 2023

View reviewed changes

src/private/translator/avp_to_cedar.rs Show resolved Hide resolved

camarcio reviewed Nov 29, 2023

View reviewed changes

ShiromMakkad reviewed Nov 29, 2023

View reviewed changes

src/private/sources/policy/core.rs Outdated Show resolved Hide resolved

Update to use http::StatusCode enum again

35294fc

jiaqiwl reviewed Nov 29, 2023

View reviewed changes

src/private/translator/avp_to_cedar.rs Outdated Show resolved Hide resolved

Remove http package and create own StatusCode enum

14669eb

GurvirDehal commented Nov 30, 2023

View reviewed changes

src/public/client.rs Outdated Show resolved Hide resolved

ericox reviewed Nov 30, 2023

View reviewed changes

src/private/sources/cache/policy.rs Outdated Show resolved Hide resolved

src/public/client.rs Outdated Show resolved Hide resolved

This was referenced Nov 30, 2023

Added SemVer validation check into CI workflow for non-release PR #35

Merged

Improved unit tests coverage #34

Merged

Address comments

8bc3467

camarcio approved these changes Nov 30, 2023

View reviewed changes

GurvirDehal merged commit f56df00 into main Nov 30, 2023
2 checks passed

GurvirDehal deleted the upgrade-dependencies branch November 30, 2023 22:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgraded AWS dependencies to 1.0 #33

Upgraded AWS dependencies to 1.0 #33

GurvirDehal commented Nov 28, 2023 •

edited

Loading

github-actions bot commented Nov 28, 2023

github-actions bot commented Nov 29, 2023

github-actions bot commented Nov 30, 2023

ericox left a comment •

edited

Loading

GurvirDehal commented Nov 30, 2023

github-actions bot commented Nov 30, 2023

camarcio left a comment

Upgraded AWS dependencies to 1.0 #33

Upgraded AWS dependencies to 1.0 #33

Conversation

GurvirDehal commented Nov 28, 2023 • edited Loading

Description of changes

Issue #, if available

Checklist for requesting a review

Testing

Disclaimer

github-actions bot commented Nov 28, 2023

github-actions bot commented Nov 29, 2023

github-actions bot commented Nov 30, 2023

ericox left a comment • edited Loading

Choose a reason for hiding this comment

GurvirDehal commented Nov 30, 2023

github-actions bot commented Nov 30, 2023

camarcio left a comment

Choose a reason for hiding this comment

GurvirDehal commented Nov 28, 2023 •

edited

Loading

ericox left a comment •

edited

Loading