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

Question: Could we statically represent credentials provider using ADTs such that configuration library can report/document them back? #144

Open
afsalthaj opened this issue Nov 25, 2020 · 6 comments
Labels
enhancement New feature or request

Comments

@afsalthaj
Copy link

No description provided.

@vigoo
Copy link
Collaborator

vigoo commented Nov 28, 2020

I could definitely create more ADT wrappers for some AWS types such as the credentials provider but I don't understand how this would help.

Currently we have this:

val credentialsProvider: ConfigDescriptor[AwsCredentialsProvider] = {
      val defaultCredentialsProvider: ConfigDescriptor[AwsCredentialsProvider] =
        string.xmapEither(
          s =>
            if (s == "default") Right(DefaultCredentialsProvider.create())
            else Left("Not 'default'"),
          {
            case _: DefaultCredentialsProvider => Right("default")
            case _                             => Left("Unsupported credentials provider")
          }
        )
      val anonymousCredentialsProvider
          : ConfigDescriptor[AwsCredentialsProvider] = string.xmapEither(
        s =>
          if (s == "anonymous") Right(AnonymousCredentialsProvider.create())
          else Left("Not 'anonymous'"),
        {
          case _: AnonymousCredentialsProvider => Right("anonymous")
          case _                               => Left("Unsupported credentials provider")
        }
      )
      val staticCredentialsProvider: ConfigDescriptor[AwsCredentialsProvider] =
        awsCredentials.xmap(
          creds => StaticCredentialsProvider.create(creds),
          _.resolveCredentials()
        )

      defaultCredentialsProvider <> anonymousCredentialsProvider <> staticCredentialsProvider
    }

If it would not use the underlying credentials provider directly then the only difference would be that instead of string("x").xmapEither(...) we could write it in a more concise way as string("x")(X.apply, X.unapply) but that gives the same underlying XmapEither wrapped descriptor. Your new enumeration helper in zio/zio-config#451 also does this. So maybe there is something I don't see here :)

On the other hand there are some descriptors marked with NOTE: Cannot extract conditions without reflection comments which does not have a way to convert back to config value, for those it would definitely make sense to create a wrapper.

@vigoo vigoo added the enhancement New feature or request label Nov 29, 2020
@afsalthaj
Copy link
Author

afsalthaj commented Nov 29, 2020

Static representation using ADTs would document the constants in your markdown even if there is Xmap node involved in it.
Having an Xmap doesn't necessarily mean you lose the informations because it's all going to be RunTime.

Example of such a markdown for a case class that uses ADTs is given below:

sealed trait Auth

@name("default")
case object Default extends Auth

@name("credentials")
case class Credentials(username: String, password: String) extends Auth


case class MyConfig(auth: Auth)

Markdown will say:

fieldName. format. description
auth all-of[Auth]

Auth

fieldName format description.
credentials all-of [Credentials]
Constant value "default" value of type string

Credentials

fieldName format description
username string value of type string
password string value of type string

On top of this, if you want to generate a sample config using zio-config-gen , pushing more things towards static boundaries would help. For example, consider the case where you end up writing the below write function in xmap.

string("auth")
  .xmapEither(
    s => if (s == "default") Right(Default) else Left("Supports only default"), 
    case Default => Right("default")
  )

(yes, zio-config-magnolia would be a better candidate here to avoid the question of what if I do case Default => "bla")

The above code would mean that when writing a randomly generated config ensure that value is going to be default, and not anything random.

Long story short, generally prefer pushing it to ADTs whenever there is a coproductish use case.

@afsalthaj
Copy link
Author

I agree that in the markdown, not having name for that constant value Default can be sort of making us feel mmm. However its a view thing. Point is we are grabbing that info.
Also table is exposed for any other view. We can make it work with Json as well in near future.

Along with it, the upcoming zio-config-gen (PR raised) would allow the user to edit a randomly generated (yet mostly correct) config. And the correctness here would be a function of how much we represent things statically.

@afsalthaj
Copy link
Author

afsalthaj commented Nov 29, 2020

If you are happy, I can sort of work on the config side of things. A few changes are already in my fork. Give me this week time though :)

@vigoo
Copy link
Collaborator

vigoo commented Nov 29, 2020

Thanks for the detailed explanation. I'm not definitely against introducing more wrappers here but my point is that the hand-written descriptors should be equally powerful than the derived ones.

Now I checked this in zio-config to understand better your point and realized that this is not the case. There is a ConstantString PropertyType in DerivationUtils and so that's a special case only used by derivation. I would expose constant beside the other config descriptors to all users and then it can be used in my hand-written descriptors too :) I will open a zio-config issue about this so we can discuss there if you agree or not.

I also checked the config generator PR and looked to me that it does not contain the part yet that would handle this and I have some similar to above concerns related to it, I'll write a comment soon about it there.

@afsalthaj
Copy link
Author

afsalthaj commented Nov 29, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants