Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Feat/scaleway.com #899
base: master
Are you sure you want to change the base?
Feat/scaleway.com #899
Changes from 12 commits
75d1a15
740aa38
ac47230
1e80577
ea37d4a
49128bc
1ff7a34
5576557
0398397
fd3ee27
6f66a08
0b9af33
d13729d
4ab47f7
d21fd33
f67e76f
3642186
65684fa
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should not need this, it should be deducted from the public IP address obtained and "ip_version"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uhm... again can you help me here? Do you have already a function that is able to distinguish between the two?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just found it in one of your comment below. Will change this in a moment!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
field_type is still in the documentation 😉 Please remove 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
argh sorry... missed the second entry...
Now it is removed for good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should not need this, it should be deducted from the "domain" field
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please help me out here. How should I extract this? The only way I can imagine is to count how many
.
are in the domain string, and if there are two take the first part of the domain string before the first.
asname
(according to the scaleway API). Is that correct?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The upper layers of ddns-updater takes care of extracting it:
ddns-updater/internal/params/json.go
Line 231 in 20ac110
It's a bit more complicated than the number of dots, it uses the "effective TLD + 1" mechanism to do this (i.e. some effective TLDs are
com.
, others areduckdns.org.
)So just use the domain and owner strings from calling layers 😉 - it's already parsed and ready to use for you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, it took me a while to understand what you mean and especialyl how this maps to the scaleway api, but I think I sorted out! I removed the field_name both in the code and documentation, and tested that everything works correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit rename to
fieldType
(Go convention is no snake_case 🤷 Only camelCase)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true! Should have noticed... changed accordingly!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there any useful information in the body? if it's just for the error case, let's use instead:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the body looks something like
I think there is nothing useful, let me know if you disagree. Otherwise I will apply your change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if you send a request with the wrong secret key or an owner that doesn't exist, what's the json body you get in the response? It might be a standardized error format which we could parse and use to build a better error to log to the user 😉 Not compulsory, but nice to have if you have the time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the secret key is wrong the response is:
If the owner doesn't exist then the record with the right domain is added. The response is indistinguishable from one where the owner already exists.
So, I changed the code into:
Is that ok?