If less than 10 keys exist, atleast 1 key can be made, but if 10 keys…

[AumJavalgikar]

… already exist then initialising keys never makes any new ones, regardless if some are deleted.

[doluk]

Hey thank you for your pr! But how is your code supposed to fix that?

[AumJavalgikar]

if number of keys in coc api is max (10), after the code that deletes the keys matching name but not IP address, get the total number of keys again, now when comparing len(keys) < KEYS_MAXIMUM, if any keys were deleted, we will create new keys, which previously did not happen as we were comparing length of the keys we fetched previous to any possible deletion.

[csuvajit]

I think the main problem was here.

            if len(self._keys) < self.key_count:
                for key in (k for k in keys if k["name"] == self.key_names and ip not in k["cidrRanges"]):
                    LOG.info(
                            "Deleting key with the name %s and IP %s (not matching our current IP address).",
                            self.key_names, key["cidrRanges"],
                    )
                    await session.post("https://developer.clashofclans.com/api/apikey/revoke", json={"id": key["id"]})

It would work fine with less than 10 keys because there is enough slot to create new keys.

But if there are 10 keys, len(keys) would still be 10 because keys are being deleted from the account, but not from the keys variable.

            while len(self._keys) < self.key_count and len(keys) < KEY_MAXIMUM:

And this loop never runs.

@AumJavalgikar is basically re-fetching keys. This could be done by removing the deleted key from keys variable too but that works anyway.

[AumJavalgikar]

yes, removing the deleted key from the keys list is better than an extra http request😊

[doluk]

also it should be if len(self._keys) =< self.key_count or? because for 10 existing keys we want the same behavior?

[AumJavalgikar]

I think that part is fine and needs no change, what may happen is in one iteration it will create the 10th key and since condition is true it will attempt making an 11th key and fail. edit: my mistake, I forgot there's an if statement above, thought you were talking about the while loop.

[mathsman5133]

I agree with the removing the keys from the existing list rather than an extra HTTP request. I wonder if there is a case for getting a "most up to date version of current keys", however, in case removing one failed for some reason. I could be persuaded either way.

                if len(keys) == 10:

I think this line could in theory lead to an issue, where you had 9 keys prior to deleting, the if block above would run and delete some keys because you wanted to make more than 1, but then they wouldn't get refreshed because you didn't start with 10. I'm not sure if that makes sense.

Regarding doluk's comment, if len(self._keys) =< self.key_count, isn't it fine if the number of keys matches the number of keys you want? I don't see that as an issue - if you have 0 keys already, want to make 4, so now len(self._keys) is 4, the same as key count, I wouldn't expect this block of code to run?

Either way, if anyone wants to pick this up and address the couple of points mentioned here and in above comments that'd be much appreciated!

[AumJavalgikar]

How about something like

resp = await session.post("https://developer.clashofclans.com/api/apikey/revoke", json={"id": key["id"]})
if resp.status == 200:
    keys.pop(-1)

I don't think it matters which key we're dropping since we're only concerned with it's length, but if you do want to drop the one that isn't matching IP that would need a for loop that could slow things down.