Skip to content
This repository has been archived by the owner on Oct 6, 2022. It is now read-only.

Hello world #1

Merged
merged 6 commits into from
Jan 3, 2021
Merged

Hello world #1

merged 6 commits into from
Jan 3, 2021

Conversation

jeroenvandijk
Copy link
Collaborator

@jeroenvandijk jeroenvandijk commented Jan 2, 2021

I've added a first implementation. A few notes:

  • My first tests are succesful except for aws/doc, see below. See script/test for a working example
  • I didn't have the right setup for the tools.deps uberjar creation used in script/compile so I used the setup from graalvm-clojure.
  • This setup uses the graalvm configuration files generated via lein native-config. I only added three lines there
  • Out of the box this pod supports the same credentials types as cognitects library. However customization is a bit harder due to the pods communication layer. For instance, I would need support for credential_process, something that is supported by other AWS tools, but this requires the implementation of a custom credential provider. This interface is not yet accessible outside of the pod. (some background here)

@jeroenvandijk jeroenvandijk requested review from borkdude and lispyclouds and removed request for borkdude and lispyclouds January 2, 2021 20:16
@lispyclouds
Copy link
Member

Hey @jeroenvandijk this looks really awesome and kudos to the effort!
One suggestion i have is to maybe use https://clojuredocs.org/clojure.core/with-out-str to proxy the call to (aws/doc ...) and capture the output of the call and send it back as a pod response? We need a shim method in the pod for this. What say?

script/test Outdated
(if (and (System/getenv "AWS_ACCESS_KEY_ID") (System/getenv "AWS_SECRET_ACCESS_KEY"))
(println (aws/invoke s3 {:op :ListBuckets}))
(println "No credentials"))

Copy link
Member

Choose a reason for hiding this comment

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

You could use an actual deftest like this to have better feedback

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah you mean the use this for a proper unit test, right? I didn't think the unit test setup through yet. Should I look at one of the other pods for an example?

Copy link
Member

Choose a reason for hiding this comment

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

yes and i think they are kinda simple and if you have better ideas for tests for the AWS ones that would be even more awesome!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok cool, thanks! I'll have a better look tomorrow. Getting late here

Copy link
Contributor

Choose a reason for hiding this comment

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

@jeroenvandijk Here is an example bb script with tests included:

https://github.com/babashka/pod-babashka-sqlite3/blob/main/test/script.clj

This can be hooked up in CI

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! I've added tests now

(defn ops [client]
(aws/ops (get-client client)))

(defn doc [client op]
Copy link
Member

Choose a reason for hiding this comment

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

This can be surrounded by the with-out-str shim i think

Copy link
Member

Choose a reason for hiding this comment

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

Then you can reply it as a map containing the out key like this That should handle it correctly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've captured the out value one level above 648a8c1 It's pretty close to the implementation of with-out-str

@jeroenvandijk
Copy link
Collaborator Author

Now with tests. I also added some missing namespaces. Everything seems to be working. Note that I haven't put it through serious AWS usage yet, but I believe it's a proper start.

Open items:

  • with-out-str doesn't work for the output that comes from the pod. @borkdude Do we need to use something else then System/out or should I test it differently?
  • migrate from leiningen to tools.deps like the initial setup by @borkdude (I need some assistence here)
  • Setup CI script. @borkdude I assume you want to use CircleCI? I can otherwise look into a Github Action setup.

@jeroenvandijk
Copy link
Collaborator Author

Oh and in addition I'm still thinking about how to integrate the credential interface for custom setups. But it's already usable without this addition through AWS environment vars.

  • Add credentials interface

@borkdude borkdude merged commit 3a39681 into main Jan 3, 2021
@borkdude borkdude deleted the hello-world branch January 3, 2021 15:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants