-
Notifications
You must be signed in to change notification settings - Fork 44
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
Existing record attributes get set to empty upon a record change. #559
Comments
@VNRARA Thanks for the reporting and sorry for the trouble. It was designed not to do this. I need more information to track down the bug (if any). Which attributes were removed? What are the log and configuration (but remember to remove sensitive content such as your API tokens)? |
@VNRARA Thank you. A proper fix needs the upstream to change their code and I have created an issue at cloudflare/cloudflare-go#1371. I can also implement a hack that retrieves your existing comment before updating it, and maybe I will release a new version with the hack soon. Or, you can use version 1.8.2 or below before the upstream library introduced the current behavior. Some background: I have noticed this design issue of the upstream library some time ago but didn't double check their final solution after getting busy at my primary job. It seems the design bug of the upstream library started to cause clearing (what you observed) at the version 0.60.0, and that means the updater of versions after 1.8.3 were all affected. Sorry about the trouble. |
I'll keep using the latest version and fix the comments some later time. |
@VNRARA I tried to implement the hack and it messed up the current test cases a bit. I prefer to wait for the upstream to fix it, but I will implement the hack (with appropriate testing) if the upstream decides not to do anything. |
@VNRARA Progress: I made a PR to the upstream repo: cloudflare/cloudflare-go#1393. Ideally the DNS updater does not have to change if this PR is merged. |
@VNRARA Progress: my PR was merged, and I'll release a new version once the dependency is updated. |
@VNRARA I believe the bug was fixed. Before I release the next official version, could you confirm that the |
Hey thanks for keeping me up to date. I'm unsure what how you'd like me to test the |
@VNRARA The current installation guide told you to use the image |
I'll try to report back withing 8 hours. |
Srry for being late: It seems to work fine. |
@VNRARA Thanks for the confirmation. |
@VNRARA The new version with the fix has been published. It is recommended to switch back to |
Existing record attributes are removed when records are updated. I'd like it not to.
The text was updated successfully, but these errors were encountered: