Skip to content
This repository was archived by the owner on Oct 24, 2025. It is now read-only.

fix: remove stale resources from state#91

Merged
kimdre merged 9 commits into
mainfrom
fix/90-remove-staled-resources
Jul 12, 2024
Merged

fix: remove stale resources from state#91
kimdre merged 9 commits into
mainfrom
fix/90-remove-staled-resources

Conversation

@kimdre
Copy link
Copy Markdown
Member

@kimdre kimdre commented Jul 12, 2024

Stale resources will now get deleted from the state on Read execution instead of showing a Warning: Resource Not Found

@kimdre kimdre added Kind/Bug Something isn't working as expected Kind/Enhancement Improve existing functionallity labels Jul 12, 2024
@kimdre kimdre self-assigned this Jul 12, 2024
@kimdre kimdre linked an issue Jul 12, 2024 that may be closed by this pull request
@kimdre kimdre requested a review from jkroepke July 12, 2024 08:30
@jkroepke
Copy link
Copy Markdown
Member

Is this testable?

@kimdre
Copy link
Copy Markdown
Member Author

kimdre commented Jul 12, 2024

Is this testable?

Should be no problem. I will add a test.

@kimdre kimdre changed the title fix: remove staled resources from state fix: remove stale resources from state Jul 12, 2024
@kimdre
Copy link
Copy Markdown
Member Author

kimdre commented Jul 12, 2024

@jkroepke The TestAccPrimaryServer_StalePrimaryServersResources test fails with a runtime error.
Could you maybe check if you can find the error?
The other two test are working fine.

@jkroepke
Copy link
Copy Markdown
Member

jkroepke commented Jul 12, 2024

Leider unsauber geschrieben.

switch resp.StatusCode {
case http.StatusNotFound:
return nil, nil

gibt auch nil bei not found zurück, aber auch keinen Fehler.

Daher Nil Pointer panic

@kimdre
Copy link
Copy Markdown
Member Author

kimdre commented Jul 12, 2024

Should be fixed now.

@kimdre kimdre merged commit aae77f1 into main Jul 12, 2024
@kimdre kimdre deleted the fix/90-remove-staled-resources branch July 12, 2024 11:22
return nil
})
if err != nil {
if err != nil && fmt.Sprint(err) != fmt.Sprintf("primary server %s not found", state.ID.ValueString()) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@kimdre das ist echt schlechtes go :D

besser wäre es, im api packages

var ErrNotFound = errors.New("not found")

dann beim error

return nil, fmt.Errorf("primary server %s: %w", id, ErrNotFound)

und hier in der condition:

if err != nil && !errors.Is(err, api.ErrNotFound))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

undd as ganze auch für zones und records :D

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Jaja hatte da keine Zeit mehr für ne schöne Lösung 😂
#92

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Kind/Bug Something isn't working as expected Kind/Enhancement Improve existing functionallity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Resource Not Found does not remove record from state.

2 participants