Feature: Added RRSet comments support #87
Conversation
PetrusHahol
changed the title
|
Import support will be added soon. |
PetrusHahol
changed the title
powerdns/client.go
Outdated
| @@ -491,26 +517,13 @@ func (client *Client) ListRecords(zone string) ([]Record, error) { | |||
|
|
|||
| err = client.Cache.Set([]byte(zone), cacheValue, client.CacheTTL) | |||
| if err != nil { | |||
| return nil, fmt.Errorf("The cache for REST API requests is enabled but the size isn't enough: cacheSize: %db \n %s", | |||
| DefaultCacheSize, err) | |||
| fmt.Printf("[WARN] The cache for REST API requests is enabled but"+ | |||
There was a problem hiding this comment.
@mbag
Have a look at this change. I think it's better to through not an error but a warning message here.
If it needed, I can create it as a separate MR.
There was a problem hiding this comment.
@PetrusHahol I think it should return error (for example JSON marshaling above returns error), because user enables caching and expects objects to be cached. But if there is error and they are not cached, however, the execution continues with only warning. This might not be something they want to pass.
Also, I'm wondering now if the error described is only possible error, i.e. if it wouldn't be better to just do return nil, err as in other error checks. Because, if in fact, the client.Cache.Set fails for some other reason, it might get obscured by our custom error message. But this was my error, that I haven't spotted before merging.
There was a problem hiding this comment.
oh, I forgot to ask. Why do you think it's better to just log warning, and continue with the execution?
There was a problem hiding this comment.
Hmm.. Actually I also think it may lead to some different error. I will change it.
I have just totally otherwise picture of it. In case users use cache and something went wrong with it, we can still continue execution and in a meanwhile let them know that something crashed. User can immediately stop execution. But maybe you're right, it might lead to DDoS of PowerDNS API, like if it would be without caches enabled.
There was a problem hiding this comment.
Hello guys, any update with the "comment" paramenter ?
|
bump! |
Solves #62.