renamed server configuration variable to server_url
#105
Conversation
beniwohli
force-pushed
the
feature/server-config-rename
branch
from
066086c to
6c32323
Compare
This aligns the Python agent to agents in other languages This PR also simplifies the logic for sending data, and uses a test server instead of mocks for many tests
beniwohli
force-pushed
the
feature/server-config-rename
branch
from
6c32323 to
480fea9
Compare
| for processor in self.processors: | ||
| transaction = processor(self, transaction) | ||
| transactions.append(transaction) | ||
| if self.instrumentation_store: |
There was a problem hiding this comment.
this if is related to one of the subtle bugs you mentioned? isn't possible to init instrumentation_store to something not None?, like eg. in a way that if you can get_all() on it you get an empty list...
There was a problem hiding this comment.
The problem here was if we had a configuration error, we would bail out early in __init__, before the instrumentation_store attribute was even set. Then when we try to collect transactions, the code explodes. Due to mocking, that code path was never hit in tests.
I don't think it's worth the effort to initialize the instrumentation_store to a DummyStore or somesuch, only to change it to the real store a few lines down (if the config validates), tbh.
| for url, transport in self._transports.items(): | ||
| for url, transport in list(self._transports.items()): | ||
| transport.close() | ||
| self._transports.pop(url) |
There was a problem hiding this comment.
hmm good find! (i think...) 😁
This aligns the Python agent to agents in other languages
This PR also simplifies the logic for sending data, and
uses a test server instead of mocks for many tests