ResilientSession: do not log request data on ConnectionError #928
Conversation
|
@Flupp would you be able to write a test for it? since CI is up and running ... |
|
I tried coming up with a test for checking that While doing that I noticed and fixed another problem; see ef184c7. |
|
Addressed CI linting errors. |
|
Rebased and fixed merge conflicts introduced by d1f244c. |
The request data may contain secrets, e.g., a plaintext password when using basic auth. Therefore the request data shall not be logged.
The test checks that no sensitive data is written to the log in case of a connection errors.
|
Rebased and fixed timeout issue in test by introducing configurable maximal retry delay in b331f6a. |
| @@ -129,7 +130,7 @@ def __recoverable( | |||
| msg = "Atlassian's bug https://jira.atlassian.com/browse/JRA-41559" | |||
|
|
|||
| # Exponential backoff with full jitter. | |||
| delay = min(60, 10 * 2 ** counter) * random.random() | |||
| delay = min(self.max_retry_delay, 10 * 2 ** counter) * random.random() | |||
|
the codacy fail can be ignored i think https://app.codacy.com/gh/pycontribs/jira/pullRequest?prid=7435641 |
|
Do I have to do something to ignore the Codacy warning (like adding some annotation in the code) or will you just silently ignore the warning? |
@ssbarnea should "approve" it in the tool i think |
|
FYI, I removed codacity, too much false positives. |
…ibs#928) * ResilientSession: make maximal retry delay configurable * ResilientSession: do not log request data on ConnectionError The request data may contain secrets, e.g., a plaintext password when using basic auth. Therefore the request data shall not be logged. * test that no sensitive data is written to the log The test checks that no sensitive data is written to the log in case of a connection errors.
The request data may contain secrets, e.g., a plaintext password when using basic auth. Therefore the request data shall not be logged.
In version 2.0.0, the basic auth login credentials were logged when a ConnectionError was triggered by an invalid SSL certificate. With current master this is not the case anymore, however, the problem might still occur under other circumstances.