Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stabilize docker readings behavior #16

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Stift
Copy link

@Stift Stift commented Apr 2, 2021

Sometimes docker doesn't return any values in the headers. The header just contains the timespan, but not the actual value. In Issues #15 and #6 the behavior is visible and should be effectively mitigated by this PR.
With this pull request we do the following:

  • decode the JWT token and extract the limit contained in there
  • store the current value to re-use it on the next run when it occurs that the header returned from docker is not complete

use previous values when current values aren't in headers
@mstein11
Copy link
Contributor

@Stift thanks for contributing.

I think your changes would not fix the issue described in #15 because the method limit_extractor is still called and will still fail if the header string is as described in the issue. Is this possible or am I mistaken here?

However, I think your pull request is still interesting for two reasons:

Firstly, it might alleviate #6 because it caches a previous result from the docker hub api which could be used if we have no response from the api. However, if this is something that we want do (lets see what @immanuelfodor thinks about this), we should also always cache the last success value. Currently you only cache the value once when the container is startet and use this value for all cases where it is failing.

Secondly, you extract the remaining requests information from the jwt token. I didn't know that this is possible, is it as accurate as the information in the head requests? If so, we might want to only use the information from the jwt token. This would save us (and the docker hub api) the head request. If we want to do this, we could remove most of the code that is called after get_token. @Stift What do you think?

@immanuelfodor
Copy link

Although the DockerHub API seems to be fixed, I think caching the last successful read as a fallback solution is acceptable in case of future API problems. For details, see this comment: #6 (comment)

@mstein11
Copy link
Contributor

@immanuelfodor thanks!

So lets go with the caching approach. @Stift, since you already wrote most of the code, do you want fix the above mentioned issues, afterwards I will merge :-)

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.

3 participants