-
Notifications
You must be signed in to change notification settings - Fork 2
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
🐛 space and org data source #111
🐛 space and org data source #111
Conversation
Signed-off-by: Matthias Theuermann <[email protected]>
9b64e0e
to
bec3f39
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @mati007thm I added a few comments so that I can understand the change better.
@chris-rock I looked over your comments and would like to clarify, why @Pauti and I made certain changes to the code: As a user I would want to call the data source with either ID or MRN but never with both at the same time, because as a normal user I do not know which of the provided values the returned result would be based of.
|
Thank you @mati007thm for the explanation. In this case we need a hack schema validation eg. as defined in #92 (comment). The problem with the current implementation in this PR is that it would check it at runtime and that is too late. Please switch it to validator check instead. |
Signed-off-by: Matthias Theuermann <[email protected]>
@chris-rock I added the suggested validations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @mati007thm for adding the validators. Great addition.
@@ -92,9 +108,10 @@ func (d *SpaceDataSource) Read(ctx context.Context, req datasource.ReadRequest, | |||
spaceMrn := "" | |||
if data.SpaceMrn.ValueString() != "" { | |||
spaceMrn = data.SpaceMrn.ValueString() | |||
} else if data.SpaceMrn.ValueString() != "" { | |||
spaceMrn = orgPrefix + data.SpaceID.ValueString() | |||
} else if data.SpaceID.ValueString() != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch
We found some errors when testing the space data source and fixed them in this pull request + some minor changes to the org data source