mozilla-releng / firefox-infra-changelog

Automated tool which builds a changelog of commits happening on git and hg that could affect Firefox CI Infra.
2 stars 8 forks source link

[FICGitHub] Issues with handling Tokens #428

Closed danlabici closed 5 years ago

danlabici commented 5 years ago

This is what @raduiman found while looking on the change made by @zsoltfay

Looking at the new code implementation, noticed the following:

* if token_handler is the main method, how effective is to get the OS.VAR from the system and re-authenticate to github each time we check if the limit request was reached?

* where is initialized 'nr_of_tokens'? also the script failed in the initialization phase because that parameter is missing >self.token_handler()<. can it be replaced with len(self._available_tokens)?

* limit_checker(self) method returns False if the limit requests < 5 and nothing == False if the limit requests > 5 (???) . we can change this method to return True in case of the limit request is not reached or call the method to switch the token and re-login with the new one.
  Also this part of code is not working well. It will change the token any time:
     if not self.limit_checker():
          # return True if token switch successful and credit limit was reset
          if self._switch_token():
               return True
          else:
               self._get_reset_time()
     else:
               return True

if not self.limit_checker(): -> returns True (not NoneType = True) in case of limit requests is NOT reached if self._switch_token(): -> this will change the token

self._get_reset_time() -> this method returns the reset time. this should be print out or remove from the return if the info from the log is enough

Also the following line should be removed:

print(os.environ.get(self._available_tokens[self.token_counter]))

@Akhliskun , @zsoltfay