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

feat: Python MPL #805

Merged
merged 125 commits into from
Oct 4, 2024
Merged

feat: Python MPL #805

merged 125 commits into from
Oct 4, 2024

Conversation

lucasmcdonald3
Copy link
Contributor

Issue #, if available:

Description of changes:

Squash/merge commit message, if applicable:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

lucasmcdonald3 and others added 30 commits February 5, 2024 14:45
* feat: remove quantifier-syntax and pin prettier version
#195)

Applies several fixes/improvements in order to work with newer Dafny versions.

* Adds `smithy-dafny` as a submodule so that we can lock down the exact commit used to generate code, and use the tool in CI.
* Updates the shared makefile with similar improvements to smithy-dafny's (hook up `--library-root` and `--patch-files-dir`, generate dependencies first)
* Regenerates the checked-in code using a newer `smithy-dafny` to abstract away from the changes in Java TypeDescriptors when constructing datatypes in Dafny 4.3. This includes adding some helper methods to Dafny code for the benefit of Java external code, in the same style as smithy-lang/smithy-dafny#301 did.
  * Also updated various `__default` classes to use the above to avoid constructing Dafny datatypes directly. 
  * Regenerating means that the effect of smithy-lang/smithy-dafny#311 on C# code shows up too.
  * Introduced `InternalResult<T, R>` to replace some internal-only uses of Dafny's compiled `Result` type, to avoid even more Dafny helper methods. 
* Leverage the `smithy-dafny` `<library-root>/codegen-patches[/<service>]` feature to extract out manual patch files.
  * This allows building with a newer version of Dafny to work despite having to regenerate code differently, in some cases by providing different patch files for different version ranges.
  * Cheated slightly by using this to conditionally remove an instance of `{:vcs_split_on_every_assert}` on `AwsKmsKeyring.OnDecrypt'` that is necessary on Dafny 4.2 but makes things work on Dafny 4.4 (which was not the intention of the patching feature, but also solves this problem much more cheaply than having to refactor the code to work in both versions :)
* Add regenerating code to CI, either to verify that it matches what's checked in, or to pick up the necessary changes to work with newer Dafny versions.

Manually verified the CI passes on the source branch with the latest Dafny nightly prerelease: https://github.com/aws/aws-cryptographic-material-providers-library/actions/runs/8039889665

Note that CI will now reject making further manual edits to generated files without capturing those edits in patch files. The error message will suggest how to update the patch files accordingly. See also https://github.com/smithy-lang/smithy-dafny/tree/main-1.x/TestModels/CodegenPatches
Bumps software.amazon.awssdk:bom from 2.19.1 to 2.25.1.

---
updated-dependencies:
- dependency-name: software.amazon.awssdk:bom
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Replaces nearly all of `SharedMakefileV2.mk` with the new `SmithyDafnyMakefile.mk` from the `smithy-dafny` submodule. What's left is just the repo-specific configuration of variables.

See #233 for a more readable diff of the existing 
`SharedMakefileV2.mk` with `SmithyDafnyMakefile.mk`, showing what targets are changing (which is very little as the extra target/improvements in the former were mostly carried over to the latter already).
* chore: add example of tail recursion
… type patches (#267)

Uses a newer version of smithy-dafny that corrects the service constructor result type in the same way that #256 fixed it with manual patches.

See smithy-lang/smithy-dafny#331 for details on why the helper functions switched back to the trait type.
Copy link

github-actions bot commented Oct 2, 2024

Detected changes to the release files or to the check-files action

Copy link

github-actions bot commented Oct 2, 2024

Changes to the release files or the check-files action requires 2 approvals from CODEOWNERS

Copy link
Contributor

@ajewellamz ajewellamz left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

github-actions bot commented Oct 3, 2024

@lucasmcdonald3 and @ajewellamz, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

)
)

def GetPublicKey(dafny_eccAlgorithm, dafny_privateKey):
Copy link
Contributor

Choose a reason for hiding this comment

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

I didnt catch this before but the function is missing the

else:
            return default__.CreateExternEccKeyGenFailure(
                    _smithy_error_to_dafny_error(
                        ValueError(
                            "SM2 not supported."
                        )
                    )
                )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch -- I'll open a PR to address this after merging into main, if that's alright

Copy link

github-actions bot commented Oct 4, 2024

@lucasmcdonald3 and @josecorella, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

Copy link

github-actions bot commented Oct 4, 2024

@lucasmcdonald3 and @lucasmcdonald3, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

3 similar comments
Copy link

github-actions bot commented Oct 4, 2024

@lucasmcdonald3 and @lucasmcdonald3, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

Copy link

github-actions bot commented Oct 4, 2024

@lucasmcdonald3 and @lucasmcdonald3, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

Copy link

github-actions bot commented Oct 4, 2024

@lucasmcdonald3 and @lucasmcdonald3, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

Copy link

github-actions bot commented Oct 4, 2024

@lucasmcdonald3 and @josecorella, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

Copy link

github-actions bot commented Oct 4, 2024

Detected changes to the release files or to the check-files action

@lucasmcdonald3 lucasmcdonald3 merged commit cfb2f7e into main Oct 4, 2024
193 of 194 checks passed
@lucasmcdonald3 lucasmcdonald3 deleted the python-reviewed branch October 4, 2024 22:41
josecorella pushed a commit to josecorella/aws-cryptographic-material-providers-library that referenced this pull request Oct 7, 2024
lucasmcdonald3 pushed a commit that referenced this pull request Oct 11, 2024
# [1.8.0](v1.7.0...v1.8.0) (2024-10-11)

### Bug Fixes

* **H-Keyring:** if getCache returns Error not EntryDoesNotExist, error ([#846](#846)) ([3413fcb](3413fcb))
* **Python:** Repolymorph to add orphaned shapes ([#831](#831)) ([2481dd8](2481dd8))

### Features

* Python MPL ([#805](#805)) ([cfb2f7e](cfb2f7e))
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.

10 participants