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

SCALRCORE-19663 Scalr Provider > Environment data source > Filtering by name #101

Merged
merged 10 commits into from
Dec 22, 2021

Conversation

honcharevskiy
Copy link
Contributor

@honcharevskiy honcharevskiy commented Dec 16, 2021

Changelog

Documentation

  • I have updated resource/data source documentation in docs

State

  • My changes affect the state
  • I have included a state migration and a unit test for it

…esource to obtain environment via terraform.
…ccounts and environment names in those accounts may be the same
* `name` - (Optional) Name of the environment.
* `account_id` - (Optional) ID of the environment account, in the format `acc-<RANDOM STRING>`

Arguments `id` and `name` are booth optional, but you need to specify at leas one of them to obtain scalr_environment.
Copy link
Contributor

Choose a reason for hiding this comment

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

*both and *at least and scalr_evironment write with monospace font.
All our documentation (and UI in general) doesn't use personalized approach (idk how to name this), therefore try to avoid you, your, etc.

scalr/data_source_environment.go Outdated Show resolved Hide resolved
scalr/data_source_environment.go Outdated Show resolved Hide resolved
scalr/data_source_environment_test.go Outdated Show resolved Hide resolved
scalr/data_source_environment_test.go Outdated Show resolved Hide resolved
scalr/data_source_environment_test.go Outdated Show resolved Hide resolved
scalr/helpers.go Outdated
return nil, fmt.Errorf("Environment with name '%s' not found", *options.Name)

case numberOfMatch > 1:
// todo: update the error message.
Copy link
Contributor

Choose a reason for hiding this comment

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

don't forget :)


return environment, nil
switch numberOfMatch := len(matchedEnvironments); {
Copy link
Contributor

Choose a reason for hiding this comment

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

Strange switch case. In theory, numberOfMatch can not be equal to zero, this case handles code above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This situation is possible when we have for example 3 environments with names name1, name2, and name3. For example, we are looking for an environment with the name name. Because of the implementation of the filter on the backend side, we will receive all those environments but neither of them is the right one.

CHANGELOG.md Outdated Show resolved Hide resolved
@artemvang
Copy link
Contributor

artemvang commented Dec 16, 2021

You forgot to update go.mod with new version of go-scalr. Therefore your unit-tests are failing. Replace line go-scalr dependency with something like this
github.com/scalr/go-scalr <latest-commit-in-your-branch>
and run go mod download and go mod tidy

scalr/helpers.go Outdated
var environment *scalr.Environment
type GetEnvironmentByNameOptions struct {
Name *string
Account *string `json:",omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to specify json:",omitempty", this struct will not be serialized to JSON format

@111crb111 111crb111 merged commit 94cf5f9 into develop Dec 22, 2021
@111crb111 111crb111 deleted the feature/SCALRCORE-19663 branch December 22, 2021 10:33
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.

3 participants