Skip to content

SmartCircuits new API#36

Open
jkt628 wants to merge 3 commits into
richo:mainfrom
jkt628:smartSwitch-new-API
Open

SmartCircuits new API#36
jkt628 wants to merge 3 commits into
richo:mainfrom
jkt628:smartSwitch-new-API

Conversation

@jkt628

@jkt628 jkt628 commented Apr 5, 2026

Copy link
Copy Markdown
Contributor

fixes #35

@richo

richo commented Apr 5, 2026

Copy link
Copy Markdown
Owner

Thankyou! I will try to have a look and test it today. I am pretty sus of 6cba9da, and might rebase around it.

I totally see what you're driving at with it, but the fact that the cache never purges, as well as the fact that it makes reasoning about what the integration actually sees makes me pretty skeptical. As a concrete example, it seems that sometimes they just return an empty body and if that winds up in the cache I think the whole integration will wedge itself.

@jkt628 jkt628 force-pushed the smartSwitch-new-API branch from 916bde9 to 4c9fe99 Compare April 5, 2026 15:56
@richo

richo commented Apr 5, 2026

Copy link
Copy Markdown
Owner

Oh thankyou! I was happy to go around it, but appreciate you rebasing :)

@jkt628

jkt628 commented Apr 5, 2026

Copy link
Copy Markdown
Contributor Author

Thankyou! I will try to have a look and test it today. I am pretty sus of 6cba9da, and might rebase around it.

I totally see what you're driving at with it, but the fact that the cache never purges, as well as the fact that it makes reasoning about what the integration actually sees makes me pretty skeptical. As a concrete example, it seems that sometimes they just return an empty body and if that winds up in the cache I think the whole integration will wedge itself.

this is intended only for periodic calls and the cache purges every time its API is invoked, for any client including dead and gone. i can tell you from experience it makes a big difference in HTTP traffic, especially with async.

@jkt628

jkt628 commented Apr 5, 2026

Copy link
Copy Markdown
Contributor Author

I am pretty sus

what is "sus"???

@jkt628 jkt628 force-pushed the smartSwitch-new-API branch from 4c9fe99 to efdbfcf Compare April 7, 2026 11:31
@jkt628

jkt628 commented Apr 7, 2026

Copy link
Copy Markdown
Contributor Author

fixed statistics

@jkt628 jkt628 force-pushed the smartSwitch-new-API branch 4 times, most recently from f49551d to 4f73421 Compare April 8, 2026 12:14
@jkt628

jkt628 commented Apr 21, 2026

Copy link
Copy Markdown
Contributor Author

bump

Comment thread franklinwh/client.py Outdated
case 3:
self.circuits = [None, *circuits]
case 4:
self.circuits = [None, *circuits[1:]]

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate on what the usecase for this is?

@jkt628 jkt628 Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to avoid off-by-one bugs circuits[0] is always None. the constructor accepts 3 or 4 length list, usually from a generator (length=3), but could be a copy operation (length=4). this code enforces the restriction.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. To me this just really increases the chances of off by one bugs? As a casual reader I would never really expect there to be a phantom 0th element that I need to understand or care about. I think if you are really worried about this class of problem you probably want to just make the list private and make some accessors that always do the right thing? It's not like there can be arbitrarily many of them

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed to explicit dict. note 2 is not present if circuits 1 & 2 are merged.

@jkt628 jkt628 force-pushed the smartSwitch-new-API branch from 4f73421 to e6f795b Compare July 8, 2026 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SmartCircuit new API

2 participants