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

Update from deprecated aws-sdk bindings #678

Merged
merged 4 commits into from
Nov 29, 2023
Merged

Update from deprecated aws-sdk bindings #678

merged 4 commits into from
Nov 29, 2023

Conversation

thomashoneyman
Copy link
Member

@thomashoneyman thomashoneyman commented Nov 28, 2023

Fixes #629. I manually verified that the inputs (params we pass to the s3 object) and outputs (data we receive) are unchanged in the new SDK, but I haven't actually run this code against our live bucket because the giant legacy importer run is going.

@@ -480,7 +480,7 @@
{
request = {
method = "PUT";
url = "/effect/4.0.0.tar.gz";
url = "/effect/4.0.0.tar.gz?x-id=PutObject";
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only visible change to the requests based on our integration test and I think it's harmless, but without verifying against the actual bucket I can't be completely sure.

Copy link
Member

Choose a reason for hiding this comment

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

If it's generated by the new sdk then it's probably the right thing 😄

Copy link
Member

@f-f f-f left a comment

Choose a reason for hiding this comment

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

Ah, this looks way more straightforward than the last time I tried it 😅

@@ -480,7 +480,7 @@
{
request = {
method = "PUT";
url = "/effect/4.0.0.tar.gz";
url = "/effect/4.0.0.tar.gz?x-id=PutObject";
Copy link
Member

Choose a reason for hiding this comment

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

If it's generated by the new sdk then it's probably the right thing 😄

@f-f
Copy link
Member

f-f commented Nov 29, 2023

Now that you're also online we can merge this and eventually revert it real quick if it's broken 😄

@thomashoneyman
Copy link
Member Author

thomashoneyman commented Nov 29, 2023

I was actually putting together a little script to upload, download, and delete an object in storage to be sure before I merge :) Let me give that a quick try and if it works I'll merge! I'll need about 1.5hrs before I can do it.

@f-f
Copy link
Member

f-f commented Nov 29, 2023

Ah that's lovely, thank you 🙂

@thomashoneyman
Copy link
Member Author

Ran this script to test things out, with a tarball at delete-me-1.0.0 locally:

ain :: Effect Unit
main = launchAff_ do
  -- Environment
  _ <- Env.loadEnvFile ".env"
  resources <- Env.lookupResourceEnv
  s3 <- lift2 { key: _, secret: _ } (Env.lookupRequired Env.spacesKey) (Env.lookupRequired Env.spacesSecret)

  let
    cache = Path.concat [ scratchDir, ".cache" ]

    work = do
      Storage.upload (unsafePackageName "delete-me") (unsafeVersion "1.0.0") (Path.concat [ scratchDir, "delete-me-1.0.0" ])
      versions <- Storage.query (unsafePackageName "delete-me")
      Console.log $ "Versions (uploaded): " <> String.joinWith ", " (map Version.print (Set.toUnfoldable versions))
      Storage.download (unsafePackageName "delete-me") (unsafeVersion "1.0.0") (Path.concat [ scratchDir, "delete-me-2.0.0" ])
      Storage.delete (unsafePackageName "delete-me") (unsafeVersion "1.0.0")
      versions2 <- Storage.query (unsafePackageName "delete-me")
      Console.log $ "Versions (deleted): " <> String.joinWith ", " (map Version.print (Set.toUnfoldable versions2))

  work
    # Storage.interpret (Storage.handleS3 { s3, cache })
    # Log.interpret (Log.handleTerminal Verbose)
    # Except.catch (\e -> Run.liftEffect (Console.log e *> Process.exit 1))
    # Env.runResourceEnv resources
    # Run.runBaseAff'

This works just fine with the v2 of aws-sdk:

Read file for [email protected], now uploading to delete-me/1.0.0.tar.gz...
Connecting to the bucket purescript-registry at space https://ams3.digitaloceanspaces.com with public key YMQJ24GQQ2ZOGFXZWVQB
Connected to S3!
Uploading release to the bucket at path delete-me/1.0.0.tar.gz
Uploaded [email protected] to the bucket at path delete-me/1.0.0.tar.gz
Connecting to the bucket purescript-registry at space https://ams3.digitaloceanspaces.com with public key YMQJ24GQQ2ZOGFXZWVQB
Connected to S3!
Versions (uploaded): 1.0.0
Deleting [email protected]
Connecting to the bucket purescript-registry at space https://ams3.digitaloceanspaces.com with public key YMQJ24GQQ2ZOGFXZWVQB
Connected to S3!
Deleting release from the bucket at path delete-me/1.0.0.tar.gz
Deleted release of [email protected] from S3 at the path delete-me/1.0.0.tar.gz
Connecting to the bucket purescript-registry at space https://ams3.digitaloceanspaces.com with public key YMQJ24GQQ2ZOGFXZWVQB
Connected to S3!
Versions (deleted):

However, it turns out that the Contents key is now optional in the response; with that fixed this PR works as well.

@thomashoneyman thomashoneyman merged commit 9a8ea97 into master Nov 29, 2023
15 checks passed
@thomashoneyman thomashoneyman deleted the trh/s3 branch November 29, 2023 16:04
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.

Update to v3 of AWS SDK
2 participants