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: Adding Elixir Support #88

Closed
wants to merge 20 commits into from
Closed

feat: Adding Elixir Support #88

wants to merge 20 commits into from

Conversation

tobbbles
Copy link
Contributor

@tobbbles tobbbles commented Jul 13, 2021

Hello! I'm starting to tackle being able to generate Elixir clients here, as it's supported generator. It'll be ideal to publish these generated packages to Hex and the documentation to [Hexdocs)(https://hexdocs.pm/).

Unfortunately the Elixir generator is quite primitive; so I've had to use custom templates to patch the generation of the clients' mix.ex (the Elixir equivalent of package.json).

Related issue(s)

ory/keto#418

OpenAPITools/openapi-generator#9945 (Upstream openapi-generator changes to avoid templating here)

Checklist

  • I have read the contributing guidelines
    and signed the CLA.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security vulnerability, I
    confirm that I got green light (please contact
    [email protected]) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added necessary documentation within the code base (if
    appropriate).

Further comments

TODO:

  • Add Elixir to prep + generate.sh
  • Add Elixir to release.sh
  • Create templates to ensure mix.exs is correctly configured for publishing
  • Update Docker container to container Elixir's dependencies and build tools
  • Discuss whether to create a Ory organization on https://hex.pm
  • Create https://hex.pm user account to authorize publishing packages from CI
  • Ensure release.sh can successfully publish to Hex
  • Generate clients

@CLAassistant
Copy link

CLAassistant commented Jul 13, 2021

CLA assistant check
All committers have signed the CLA.

@tobbbles tobbbles changed the title Adding Elixir SDK feat: Adding Elixir Support Jul 13, 2021
@tobbbles
Copy link
Contributor Author

For publishing to hex, we'll need a Hex (please see https://hex.pm/docs/publish -> "Registering a Hex user") user, and to make either a key (undetermined if possible) or the username/password available in CI to be consumed

Further details at https://hexdocs.pm/hex/Mix.Tasks.Hex.User.html

@tobbbles tobbbles marked this pull request as draft July 13, 2021 13:18
@aeneasr
Copy link
Member

aeneasr commented Jul 14, 2021

Thank you! I've set up the elixir user. Under which env var should I publish the key in the CI?

Regarding the templates, that's ok. One question though is if this can handle more complex specs such as the Ory Kratos spec?

Also, have you opened a PR against openapi-generator with your template changes? I would like to have these things upstream as much as possible :)

@aeneasr
Copy link
Member

aeneasr commented Jul 14, 2021

Oh and could you maybe explain the changes made to the template? :)

@aeneasr
Copy link
Member

aeneasr commented Jul 14, 2021

I installed elixir from homebrew and got mix, but mix hex is unknown - any idea how to get it working?

$ mix hex
** (Mix) The task "hex" could not be found
Note no mix.exs was found in the current directory

$  mix hex.user auth
** (Mix) The task "hex.user" could not be found
Note no mix.exs was found in the current directory

@tobbbles
Copy link
Contributor Author

👋

Thank you! I've set up the elixir user. Under which env var should I publish the key in the CI?

Regarding the templates, that's ok. One question though is if this can handle more complex specs such as the Ory Kratos > spec?

Also, have you opened a PR against openapi-generator with your template changes? I would like to have these things upstream as much as possible :)

I'm trying to double check if we can generate an API key for the Hex user and expose that as an environment variable, and using that to configure mix rather than taking username/password and doing an stdin login. There's no commands under mix help hex.user that looks to support this, so I'm digging a little deeper.

For the templates, I'm struggling to get the Dockerfile to build due to Alpine, pip's cryptography package, and requiring a Rust compiler. Are you able to replicate this @aeneasr? Once I'm able to build a new image I'll then look towards generating all clients, and testing those (such as you pointed out against the more complex specs.)

I'll open a PR for the openapi-generator upstream; great idea!

I installed elixir from homebrew and got mix, but mix hex is unknown - any idea how to get it working

Getting hex setup is a fun one, you'll want to run the following; to get it installed locally and then the hex subcommands should become available.

mix local.hex

Do we want to perform the elixir, mix, and mix hex setup in the build Dockerfile?

Oh and could you maybe explain the changes made to the template? :)

Changes made to Elixir templates only include modifiying the mix.exs.mustache file to add the following functions and configuration:

  defp description() do
    "{{appDescription}}"
  end
defp package() do
    [
      name: "{{#underscored}}{{packageName}}{{/underscored}}",
      files: ~w(lib mix.exs README* LICENSE*),
      licenses: ["Apache-2.0"]
    ]
  end

Note: I couldn't find

And finally the project defintion is updated to include these:

    ...
    package: package(),
    description: description(),

These fields are required by Hex when publishing a package

@tobbbles
Copy link
Contributor Author

Good news! Hex uses the env var HEX_API_KEY to overwrite the API keys, so we can generate a token at https://hex.pm/dashboard/keys with API Read and API Write permissions, and expose it through the env var to authenticate on publish.

@aeneasr
Copy link
Member

aeneasr commented Jul 14, 2021

Awesome, thank you! I have added the API key to Circle. Also thank you for the info on the template - do you have a PR for those in openapi-generator too?

Regarding Docker - I have no clue unfortunately. I know that alpine is a bit tricky for some things but we had someone contribute an ubuntu-based image before and it really messed up everything. I wish I could help here but I have never worked with elixir before :(

@tobbbles
Copy link
Contributor Author

tobbbles commented Jul 14, 2021

Awesome, thank you! I have added the API key to Circle. Also thank you for the info on the template - do you have a PR for those in openapi-generator too?

That's over at OpenAPITools/openapi-generator#9945 - I'm not sure what their turnaround is like, but hopefully we can get these changes up-stream to make this a bit tidier

Regarding Docker - I have no clue unfortunately. I know that alpine is a bit tricky for some things but we had someone contribute an ubuntu-based image before and it really messed up everything. I wish I could help here but I have never worked with elixir before :(

Does a fresh checkout of ory/sdk and a docker build fail for you also? I can give it a shot at tackling the Docker image, I just want to verify if it's currently broken in master, or just a weird edge-case on my builds

@aeneasr
Copy link
Member

aeneasr commented Jul 14, 2021

That's over at OpenAPITools/openapi-generator#9945 - I'm not sure what their turnaround is like, but hopefully we can get these changes up-stream to make this a bit tidier

It's usually about a week, unless there's breaking changes. One thing maintainers are definitely looking at is the CI pipeline, they usually don't even look at PRs if the CI is failing :/

It takes a bit of time to get into to the CI set up, but once you have it ensures that everything works according to spec - i.e. no build errors etc.

Obv important that master is also passing before you run your changes through the pipelines. The PR checklist is pretty helpful IMO!

Does a fresh checkout of ory/sdk and a docker build fail for you also? I can give it a shot at tackling the Docker image, I just want to verify if it's currently broken in master, or just a weird edge-case on my builds

Ah I misunderstood you then - running the build now and report once it's done

@aeneasr
Copy link
Member

aeneasr commented Jul 14, 2021

Yup fails for me as well:


#14 12.63 ERROR: Could not build wheels for cryptography which use PEP 517 and cannot be installed directly
------
executor failed running [/bin/sh -c python3 -m pip install --user --upgrade setuptools wheel twine]: exit code: 1

aeneasr added a commit that referenced this pull request Jul 14, 2021
@aeneasr aeneasr mentioned this pull request Jul 14, 2021
5 tasks
@aeneasr
Copy link
Member

aeneasr commented Jul 14, 2021

#88

@tobbbles
Copy link
Contributor Author

tobbbles commented Jul 14, 2021

Rebased from sdk:master to pick up Dockerfile changes.

Will begin work on updating Dockerfile to include Erlang/Elixir runtimes to be able to release. (Since Elixir runs on the Erlang BEAM vm)

Looking at examples, it could get quite messy. 😬

https://github.com/erlang/docker-erlang-otp/blob/master/24/alpine/Dockerfile
https://github.com/erlef/docker-elixir/blob/master/1.12/alpine/Dockerfile

I wonder if it'd be possible to use a build step, and just copy the erlang & elixir runtime and commands from the official elixir image, into the sdk builder image

@aeneasr
Copy link
Member

aeneasr commented Jul 14, 2021

That could be possible by using the docker FROM instruction, but you might need to update paths etc too

@tobbbles
Copy link
Contributor Author

👋 I'm lagging on this a little bit, mostly waiting for a new openapi-generator release to be cut which include the Elixir Mix template changes, to keep this nice and clean

@aeneasr
Copy link
Member

aeneasr commented Jul 27, 2021

Nice, thank you for the update!


# Add any middleware here (authentication)
plug Tesla.Middleware.BaseUrl, "{{{basePath}}}"
plug Tesla.Middleware.Headers, [{"user-agent", "Elixir"}]
Copy link

Choose a reason for hiding this comment

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

Suggested change
plug Tesla.Middleware.Headers, [{"user-agent", "Elixir"}]
plug Tesla.Middleware.Headers, [{"user-agent", "Elixir x.x.x"}]

Would be useful to add the package version to the user agent

:application.get_key(dep, :vsn)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We no longer use the templates as our changes got merged into the upstream https://github.com/OpenAPITools/openapi-generator project; but I think this would be really good to add there with a PR perhaps 👀

Copy link

Choose a reason for hiding this comment

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

I would for sure, the more metadata we have about particular build the better, so I would recommend it

@yordis
Copy link

yordis commented Sep 19, 2021

Related to publishing, https://github.com/ueberauth/guardian/blob/master/.github/workflows/release.yml

Don't know if you figure it out, but that one uses GitHub Actions, I hope it helps

@tobbbles
Copy link
Contributor Author

Oops; I messed up some force pushes 🙈

OKAY; so, the Dockerfile now has steps and adds Elixir tooling, the openapi-generator-cli is able to generate elixir clients through ./scripts/generate.sh

@aeneasr Do you want me to add all of the generated Elixir clients in the PR, or perhaps split those out into separate PRs?

@tobbbles
Copy link
Contributor Author

I have just rebased from master and generated all Elixir clients with the latest found specs.

@aeneasr
Copy link
Member

aeneasr commented Feb 16, 2022

Thank you! Can you confirm that running the image and scripts/generate|test.sh ( https://github.com/ory/sdk#running-the-image-locally ) work? Seems like we have not set up a CI task for this yet.

@tobbbles
Copy link
Contributor Author

Building the container and running ./scripts/generate.sh (with the FORCE_VERSION and FORCE_PROJECT envvars setup) generates the clients without fail.

I'm unable to get the test script to run due to an apparent NPM permissions error, however this is reproduceable when attempting to run the oryd/sdk image (tag v0.0.48)

With regard to publish the Elixir hex packages, we could iterate on that in another PR, as it is possible to reference dependencies through git.

@tobbbles
Copy link
Contributor Author

tobbbles commented Feb 23, 2022

Update here, we need to use RAW_VERSION (no v prefix on the semver) to be a valid Mix version; running into some trouble with the openapi generator to get it to use this value, working on it :)

EDIT: SO! There's some bullsh*t behaviour in openapi-generator that uses the spec's version field, and ignores the config specifying appVersion. I see the rust generation does a sed replace of versions, I think I'll have to solve it like this in the elixir generation, too 🙃

@tobbbles
Copy link
Contributor Author

Aaand lastly the modules are generated with the project name as the module name (originally using Ory which would clash)

@hailelagi
Copy link

Hi @tobbbles I'd like to integrate ory hydra into an elixir/phoenix application, is there some way to get started with the sdk? I've browsed the comments but don't have sufficient context on the issue. How can I help/get started?

@tobbbles
Copy link
Contributor Author

tobbbles commented Feb 23, 2022

Hi @tobbbles I'd like to integrate ory hydra into an elixir/phoenix application, is there some way to get started with the sdk? I've browsed the comments but don't have sufficient context on the issue. How can I help/get started?

If you could start testing my fork by using the Mix git directive that'd be great, try to add this to your dependencies (using Kratos as an example)

{:ory_kratos, git: "https://github.com/tobbbles/sdk.git", ref: "master", subdir: "clients/kratos/elixir"}

And referencing the Kratos module, and we can see how that goes?

Edit: Correct deps referencing

@hailelagi
Copy link

fails with:

error: pathspec 'clients/kratos/elixir' did not match any file(s) known to git
** (Mix) Command "git --git-dir=.git checkout --quiet clients/kratos/elixir" failed

@piotrmsc
Copy link

piotrmsc commented Feb 24, 2022

hey, I'm not an elixir developer but can you add to the readme a small example of usage so that we can set up the environment with elixir and see how the elixir SDK actually works in practice? :) For me, Aeneas, or other non elixir devs it will help with the review ;-)

@tobbbles
Copy link
Contributor Author

tobbbles commented Feb 24, 2022

Hi all, I've made a very short sample repo to show how the SDKs can be used from Git, and roughly how to configure and use them. https://github.com/tobbbles/ory-elixir-example

The README should explain much, but we are able to reference the dependency as such https://github.com/tobbbles/ory-elixir-example/blob/main/mix.exs#L24-L25 (in future we hope to have a https://hex.pm package published, so these can be referenced and versioned from there, along with having https://hexdocs.pm documentation generated and available)

And a very simple API call here https://github.com/tobbbles/ory-elixir-example/blob/main/lib/kratos.ex#L9

We could look to build upon this example, but at as it stands now it compiles, runs, and makes calls to the Kratos Admin API. I have not produced any other examples with the SDK due to using Ory Cloud for testing purposes

but can you add to the readme a small example of usage so that we can set up the environment with elixir and see how the elixir SDK actually works in practice

I will get around to adding this as soon as I can

@piotrmsc
Copy link

Hey @tobbbles any update with how to use it Readme ?

@aeneasr
Copy link
Member

aeneasr commented May 26, 2022

@tobbbles it's been a while since I checked here 😓 we since have added a simplistic test toolchain to see if the generated code compiles (which sometimes is an issue). For rust for eaxmple we do:


rust () {
  echo "Testing Rust..."

  dir="clients/${PROJECT}/rust"
  (cd "$dir" && cargo test)
}

What would be the equivalent for elixit? Let me know and I'll add it :)

@tobbbles
Copy link
Contributor Author

tobbbles commented May 26, 2022

What would be the equivalent for elixit? Let me know and I'll add it :)

Generally the command chain we'd run is this:

mix deps.get && mix deps.compile && mix compile && mix test

this will pull the dependencies (http client, a couple misc others), compile the dependencies, compile the source code, and run tests (of which there are none, but will return a zero status code with 'There are no tests to run')

It might be worth ensuring the MIX_ENV=prod environment variable is set, too.

@aeneasr
Copy link
Member

aeneasr commented May 27, 2022

Any idea why this happens?

root@40d1bd145cb5:/project# mix
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
Segmentation fault

@aeneasr
Copy link
Member

aeneasr commented May 27, 2022

Looks like it's a problem if we need to emulate amd64 in docker

@aeneasr aeneasr closed this in 97372e5 May 27, 2022
@aeneasr
Copy link
Member

aeneasr commented May 27, 2022

@tobbbles PTAL: #189 (comment)

@achedeuzot
Copy link

@aeneasr The issue comes from the fact that elixir uses the erlang JIT compiler features that didn't work on ARM processors until version 25.* and causing qemu to crash. Erlang 25 has been released a few weeks ago (May 18th 2022) and fixes the issue so newer compiled version of Erlang + Elixir will work on ARM too 😉

@aeneasr
Copy link
Member

aeneasr commented May 30, 2022

Ah awesome, thank you for taking the time to explain this :)

ah0y pushed a commit to ah0y/kratos that referenced this pull request Sep 8, 2023
Closes ory/sdk#88

Co-authored-by: Toby Archer <[email protected]>
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.

7 participants