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.

Sign up for GitHub

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

Jump to bottom

fix: regression on scaleway provider in 0.14.0 #4039

Merged
merged 7 commits into from
Nov 21, 2023
Merged

fix: regression on scaleway provider in 0.14.0 #4039

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
811b425
fix: issue #4038
M0NsTeRRR Nov 11, 2023
bf3065e
fix: load config file
M0NsTeRRR Nov 12, 2023
fc2f5ac
feat: add regression test
M0NsTeRRR Nov 12, 2023
d6ce656
feat: add suggestion
M0NsTeRRR Nov 16, 2023
1dc4690
feat: inform the user that the profil can't be found
M0NsTeRRR Nov 16, 2023
e542a62
Update provider/scaleway/scaleway.go
M0NsTeRRR Nov 17, 2023
37a8010
fix: gofmt
M0NsTeRRR Nov 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion provider/scaleway/scaleway.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,20 @@ func NewScalewayProvider(ctx context.Context, domainFilter endpoint.DomainFilter
defaultPageSize = 1000
}
}
p, _ := scw.MustLoadConfig().GetActiveProfile()

p := &scw.Profile{}
c, err := scw.LoadConfig()
if err != nil {
log.Infof("%s", err)
} else {
tempProfile, err := c.GetActiveProfile()
if err != nil {
log.Infof("%s", err)
} else {
p = tempProfile
}
}
M0NsTeRRR marked this conversation as resolved.
Show resolved Hide resolved

scwClient, err := scw.NewClient(
Copy link
Contributor

Choose a reason for hiding this comment

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

here you overwrite the error and it's not clear why we don't care about the error.

Copy link
Contributor Author

@M0NsTeRRR M0NsTeRRR Nov 15, 2023 •
edited

Choose a reason for hiding this comment

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

I've reused the same logic from the original PR that has been reviewed but we can change this of course. I can add a log when the file can't be loaded and when the profile was not loaded too if that's okay for you.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes please, I added an example how to make it better. I hope it helps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I keep this thread as it's the one that make most sense from the original idea.
I've added warning in logging because if someone try to use config file and the file contains error he will not be warned about that and can't understand the issue, that's why I think logs are better in the end.
If you don't like the submitted code, feel free to change it as I've don't a lot of knowledge in Go and don't want to waste your time in reviewing and make it hard. Was doing it to help a bit if I can 🙂.

Regards,

scw.WithProfile(p),
scw.WithEnv(),
Expand Down
8 changes: 8 additions & 0 deletions provider/scaleway/scaleway_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,14 @@ func TestScalewayProvider_NewScalewayProvider(t *testing.T) {
}
}

func TestScalewayProvider_OptionnalConfigFile(t *testing.T) {
_ = os.Setenv(scw.ScwAccessKeyEnv, "SCWXXXXXXXXXXXXXXXXX")
_ = os.Setenv(scw.ScwSecretKeyEnv, "11111111-1111-1111-1111-111111111111")

_, err := NewScalewayProvider(context.TODO(), endpoint.NewDomainFilter([]string{"example.com"}), true)
assert.NoError(t, err)
}

func TestScalewayProvider_AdjustEndpoints(t *testing.T) {
provider := &ScalewayProvider{}

Expand Down