i added this to be compliant with the config gherkin test - usage of this will be fixed as soon as we touch the in-process provider

Don't we increase retry_delay in handle_retry(), so we wouldn't print the correct value here?

We increment the retry_delay after the sleep, so in the next iteration, it will be increased. hence that the order of execution ensures the right values. But for clarity, I'll reorder the methods.

Just for my understanding. The retry_counter seems to be incremented (and the error and retry handling exectued) in every loop iteration, even if the connection succeeds, or am I missing something?

You are right. handle_error is a bad name for what I am actually doing. Two things are hidden behind it.

1. if we hit our grace_attempts with the error_counter we emit an error event
2. if our immediate reconnect did not work, we're emitting a stale event on the first time.

just an idea. why not change retry_counter and retry_delay to private instance attributes? then you don't have to pass them in and return and can instead update them directly.

Clarity is an integral part of software development. If we put this on the class level, I am hiding important information, as the manipulation and influence of those properties are not fully transparent. Those are connection-specific properties handled and modified only during this method and submethods that are part of this logic. Hiding them on the instance or not showing their implications might be a nice shortcut for a smaller method footprint. Still, the clarity and connection of these variables to this method would be lost. Furthermore, they might also be used or seen as properties of the resolve logic. Hence, we should keep them close to their use case.

if you want to go this route, then you should extract self.retry_backoff_max_seconds from handle_retry() and make it a class method. they are tightly coupled and should be treated in the same way. handle_retry() is just an extracted logic snippet without being reused anywhere. furthermore the incrementation of the retry_counter is outside of handle_retry() function, which contradicts your statement, because it is not clear why this is done separately and not directly inside the method.

First off, I really appreciate the time and effort you put into reviewing pull requests—it’s clear you care about the quality of the work and the success of our project. That said, I wanted to share some thoughts on how we might make our collaboration on reviews a bit smoother and more effective.

Sometimes, the way your comments are phrased can come across as more directive or critical than collaborative. I know that’s likely not your intention, but it can make it a bit challenging to engage in a back-and-forth discussion. For example, it sometimes feels like suggestions are presented without much context, making it hard to fully understand your perspective or have a productive debate. Additionally, when there are differing opinions, it can feel like there’s more emphasis on following your suggestion than on finding a shared solution together.

I think we’re working on something really cool here, and it would be awesome if we could approach pull request reviews as more of a collaborative effort. To help with that, I’d recommend taking a look at this blog post, which has some great tips for making review comments more constructive and easier to follow.

I truly value your feedback, and I hope this helps make the review process feel more like a partnership. Let me know what you think, and thanks again for all the work you put into this!

TBH the scope of this variables is not something I find that important, so neither of these refactors are a blocker for me. I can see both sides of this discussion.

As far as I can see, nothing in this PR violates the flagd spec.