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

draft of get, read and refactor to readOrRegisterResource #328

Merged
merged 20 commits into from
Dec 13, 2023

Conversation

lbialy
Copy link
Collaborator

@lbialy lbialy commented Dec 5, 2023

No description provided.

@pawelprazak pawelprazak added impact/broken Something that is difficult or impossible for some people to use kind/missing We are missing a part of functionality compared to upstream area/codegen Schema to code generator area/core The SDK's core code labels Dec 5, 2023
@pawelprazak pawelprazak added this to the 0.2.0 milestone Dec 5, 2023

final case class CustomResourceOptions private[internal] (
common: CommonResourceOptions,
provider: Option[ProviderResource],
deleteBeforeReplace: Boolean,
additionalSecretOutputs: List[String],
importId: Option[NonEmptyString] // TODO should this be Id?
importId: Option[ResourceId] // TODO should this be Id?
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: the todo note appears to be fixed now :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
importId: Option[ResourceId] // TODO should this be Id?
importId: Option[ResourceId]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it isn't really, resource options require a rewrite

@@ -81,13 +95,13 @@ trait CustomResourceOptionsFactory:
customTimeouts: String | NotProvided = NotProvided, // CustomTimeouts // TODO
// resourceTransformations: List[ResourceTransformation], // TODO
// aliases: List[Output[Alias]], // TODO
urn: String | NotProvided = NotProvided, // TODO better type
urn: URN | NotProvided = NotProvided, // TODO better type
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: the todo appear to be fixed now

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
urn: URN | NotProvided = NotProvided, // TODO better type
urn: URN | NotProvided = NotProvided,

*
* If you want to understand this regex better head to: https://regex101.com/r/o2QWJ3/1
*
* Now hear me out, this regex is a bit complicated, but it's not that bad. Let's break it down. We have to adhere to this grammar:
Copy link
Contributor

Choose a reason for hiding this comment

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

I like your humor, "not that bad regex" is dark, but still funny ;)

Don't worry, one day we'll write a proper parser :)

@lbialy lbialy merged commit d71fb59 into develop Dec 13, 2023
2 checks passed
@lbialy lbialy deleted the read-and-get-resource branch December 13, 2023 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/codegen Schema to code generator area/core The SDK's core code impact/broken Something that is difficult or impossible for some people to use kind/missing We are missing a part of functionality compared to upstream
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants