#400 Implementation of GithubOrganization and GithubRepo validators
Merged 4 years ago by abompard. Opened 4 years ago by anar.
anar/fedora-hubs GithubValidators  into  develop

@@ -30,7 +30,6 @@ 

          self.assertRaises(

              ValueError, validators.Username, "nobody")

  

-     @unittest.skip("Not implemented yet")

      def test_github_organization(self):

          self.assertEqual(

              validators.GithubOrganization("fedora-infra"),
@@ -40,12 +39,14 @@ 

              validators.GithubOrganization,

              "something-that-does-not-exist")

  

-     @unittest.skip("Not implemented yet")

      def test_github_repo(self):

          self.assertEqual(

-             validators.GithubRepo("fedmsg"), "fedmsg")

+             validators.GithubRepo('/'.join(["fedora-infra",

+                                             "fedmsg"])),

+             "fedmsg")

          self.assertRaises(ValueError, validators.GithubRepo,

-                           "something-that-does-not-exist")

+                           '/'.join(["fedora-infra",

+                                     "something-that-does-not-exist"]))

  

      def test_fmncontext(self):

          self.assertEqual(validators.FMNContext("email"), "email")

@@ -0,0 +1,86 @@ 

+ interactions:

+ - request:

+     body: null

+     headers:

+       Accept: ['*/*']

+       Accept-Encoding: ['gzip, deflate']

+       Connection: [keep-alive]

+       User-Agent: [python-requests/2.18.4]

+     method: GET

+     uri: https://api.github.com/users/fedora-infra

+   response:

+     body:

+       string: !!binary |

+         H4sIAAAAAAAAA52TzW6cMBSFXyXyehjjMJ02lqruKnXVzayyGRnjgRsZ2/IP0RTl3XsNJJogVRWz

+         Aiyf7x6OfUaibQuGcHJRjfWiAHPxguwINIRXFTseq687IgYRhT8nr3FjF6MLnNJ5MbB9C7FLdQrK

+         S2uiMnEvbU8TXeQ/hu8HBLZ+oWQywYUVzcECmtVIC3TlqYu9XpmYZ0+S1eaL1dq+ImVt+3+D6IcS

+         Tc7vYNo7KagcqY2dwvTwl95yEBDidlOTaqT5cYYmcwIeiVfNZmOLDm29GnQ0Uq+cnYCpDtKDi2DN

+         doOf1EizvhUG/oj7aKgOCMnWtluZVKhWA17G7fJZNlLnYRDymqPxSioYMOw7kSs9EuPVKezB75uU

+         8hFAVGfR9LmRF6GD2hEj+rzx51TPh1+5niH6JGPyChV4850wV8JN0npHaqzzbUedC/u5GM7bFyXj

+         HpOlqNNWTkfzLlS9AGz3TOnAK1FrnLtQwf7LwsNJiR55LtUa5HmOnrOSfSxNN5fw8r1MWMmbLyzI

+         9CVxYsSARcRJjyWripIV7OnEnvjhGz98ecYZyTWf9hyLsirY4fRY8orxsnomb38BYQwDb9AEAAA=

+     headers:

+       access-control-allow-origin: ['*']

+       access-control-expose-headers: ['ETag, Link, X-GitHub-OTP, X-RateLimit-Limit,

+           X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes,

+           X-Poll-Interval']

+       cache-control: ['public, max-age=60, s-maxage=60']

+       content-encoding: [gzip]

+       content-security-policy: [default-src 'none']

+       content-type: [application/json; charset=utf-8]

+       date: ['Wed, 18 Oct 2017 14:57:08 GMT']

+       etag: [W/"26ff4247338ae3b104803156575c2cff"]

+       expect-ct: ['max-age=2592000; report-uri="https://api.github.com/_private/browser/errors"']

+       last-modified: ['Mon, 14 Mar 2016 20:31:03 GMT']

+       server: [GitHub.com]

+       status: [200 OK]

+       strict-transport-security: [max-age=31536000; includeSubdomains; preload]

+       vary: [Accept]

+       x-content-type-options: [nosniff]

+       x-frame-options: [deny]

+       x-github-media-type: [github.v3; format=json]

+       x-github-request-id: ['8483:248DD:3178E2B:58E7F3D:59E76BC3']

+       x-ratelimit-limit: ['60']

+       x-ratelimit-remaining: ['39']

+       x-ratelimit-reset: ['1508341503']

+       x-runtime-rack: ['0.020675']

+       x-xss-protection: [1; mode=block]

+     status: {code: 200, message: OK}

+ - request:

+     body: null

+     headers:

+       Accept: ['*/*']

+       Accept-Encoding: ['gzip, deflate']

+       Connection: [keep-alive]

+       User-Agent: [python-requests/2.18.4]

+     method: GET

+     uri: https://api.github.com/users/something-that-does-not-exist

+   response:

+     body:

+       string: !!binary |

+         H4sIAAAAAAAAAxXJMQ7CMAwF0Ksgs5J6YOsBGHsFFJqvNFISV7HdBfXuhfW9LzWoxgyaaRG7vcR7

+         ogclWb2hW7Qi/e2j/n4z23VmTjhQZceYcrHNP9MqjY8nu2Io3zMsxKCl54rwNzovaRNpS2YAAAA=

+     headers:

+       access-control-allow-origin: ['*']

+       access-control-expose-headers: ['ETag, Link, X-GitHub-OTP, X-RateLimit-Limit,

+           X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes,

+           X-Poll-Interval']

+       content-encoding: [gzip]

+       content-security-policy: [default-src 'none']

+       content-type: [application/json; charset=utf-8]

+       date: ['Wed, 18 Oct 2017 14:57:09 GMT']

+       expect-ct: ['max-age=2592000; report-uri="https://api.github.com/_private/browser/errors"']

+       server: [GitHub.com]

+       status: [404 Not Found]

+       strict-transport-security: [max-age=31536000; includeSubdomains; preload]

+       x-content-type-options: [nosniff]

+       x-frame-options: [deny]

+       x-github-media-type: [github.v3; format=json]

+       x-github-request-id: ['9928:248DC:1970632:35564D7:59E76BC4']

+       x-ratelimit-limit: ['60']

+       x-ratelimit-remaining: ['38']

+       x-ratelimit-reset: ['1508341503']

+       x-runtime-rack: ['0.013258']

+       x-xss-protection: [1; mode=block]

+     status: {code: 404, message: Not Found}

+ version: 1

@@ -0,0 +1,99 @@ 

+ interactions:

+ - request:

+     body: null

+     headers:

+       Accept: ['*/*']

+       Accept-Encoding: ['gzip, deflate']

+       Connection: [keep-alive]

+       User-Agent: [python-requests/2.18.4]

+     method: GET

+     uri: https://api.github.com/repos/fedora-infra/fedmsg

+   response:

+     body:

+       string: !!binary |

+         H4sIAAAAAAAAA+1Yy27jOBD8lUDXdUzbSjIZAYvZ094GuwvkNBeDlmiJE4kUSMpGIuTft0jqZQN+

+         hL4GCBybYBWLTXazu9uIZ1ESx6vv8fPTLBK0YlESbVlW6TyaRdumLNfjoFT0noutomSYIfeCqShp

+         o1LmXHjsMA0Mnn759BR/m0V0Rw1V60aVmFgYU+uEED+ol/Ocm6LZNJqpVArDhJmnsiINiWMH/7H7

+         8wGEuepYLHOEgSO2mndEHg02bdVONRWmKo9E+LUd5GjyVpal3IPlWPalhciAtIZ0LFzkgSxAtkSa

+         gsF62NKHNQTX5vOiHKol9t+aZ5ZH40gUyz4trMNBlr0CHy1RrJaOsNnoVPHacCk+L/AADTapcir4

+         Ow1jA1qDxEr7vBSHAprtcBk/D/ewltSK72j6Zk2jWMr4DsYOpDzCg9G81dZj/5lYyR4BN2xNs8p6

+         5JaWmn3MIifDYLIbmMH/rvWC0dszNpwsFv2bZUyBMrv7ybSm8P/8bg8nvvvFlPz5H3RspXodFjzr

+         qM7WB743rmpZLpj/JBxeCDCkvLK3YA6LbQk+O5dJ4c10g5Bi5KW4cFrYAUlLpj/tVTGMVsGCHRgk

+         hZThlnNgkHCtG3bVjT29WcehSe8Soqk2Popd4winaT0aGqnWPBeMBVtsIGhJH2A3ioq0CKfs8S3x

+         39yp0jxYosWCYlPKTTAH3jniCFqiC+qfEbO+RZVltPgDQsW2N0m0+IHQqBvO1cmzBAMd3i2DIw7W

+         1+NJ21mwpCJvaB7OOBDgdO2rmtP3i/nGaZ8YGUBnMynFN81tgWrksAr98w7/DTfhSDESunzhfBZy

+         ZtOTnMNtu6r4pSf7NFsHP7jSN1Lae3hMa39fzizOy7T4lozx1AfrjjnUml207vVN+bvUPPjoezxp

+         /6ipKWwEwjI1VSxUbAcn7YYiz5nP523BqMtsK6Zu8EqPBg1VaYGkLVRf2+ORiVTUuER5a+VlSJxL

+         SbNgWw4EIPNHFqrRo6fnXKPyCxbmwFO2ipdMGynCY+TIMOUV0vAtT68pDk670QFJ+0NzkbIZLcsZ

+         bqXhKcc9RWZrTwxJHwu3ikdDPopsXwyUDFc22MqKeXxLfCGXsbqUbzdFlAmFdUzFbHa/pgbJ/mqx

+         XN0v8Ld8WS2Tx0WyfP6FOU2dHcz5dr9c3C8eXxarZPGQxAs7p250MaFxU5bxywocj8niyU5BeOzu

+         Lr6hKXCiHu9qAlvhA6R1MYL+GiHJtIQ/hKQlLuGRl1y31u74bToPg7xCVqxGXjDpdfi2yRx2zVDL

+         ZzLVcy6J3Qp/x7zV9+f4IAVIZSNg/OUDhvfUIBvFozsd7FMHrPHvmymksOtSvfYOHSVGNbbUw0it

+         5G+WGj0dGwPIZOKev/KxSLRIm9sMI74Y6zTYnlHFlZJdU0fA8VGy10x0CnqxMTbgK7HEYiYzbPdp

+         3Fy314xtaVOatU+csbkMmX4pa+xu2g/46ju5YuW4bu47VrDWV9+pazJe7NZ99Z3QfD3RM0XGctC3

+         wsW6vu8kmNmjB9NHAuv+0xKiiyTx6uN/dyzNvYwWAAA=

+     headers:

+       access-control-allow-origin: ['*']

+       access-control-expose-headers: ['ETag, Link, X-GitHub-OTP, X-RateLimit-Limit,

+           X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes,

+           X-Poll-Interval']

+       cache-control: ['public, max-age=60, s-maxage=60']

+       content-encoding: [gzip]

+       content-security-policy: [default-src 'none']

+       content-type: [application/json; charset=utf-8]

+       date: ['Wed, 18 Oct 2017 14:57:10 GMT']

+       etag: [W/"38e039a20d5cd6cc6f07ddf3cecca325"]

+       expect-ct: ['max-age=2592000; report-uri="https://api.github.com/_private/browser/errors"']

+       last-modified: ['Thu, 05 Oct 2017 02:04:30 GMT']

+       server: [GitHub.com]

+       status: [200 OK]

+       strict-transport-security: [max-age=31536000; includeSubdomains; preload]

+       vary: [Accept]

+       x-content-type-options: [nosniff]

+       x-frame-options: [deny]

+       x-github-media-type: [github.v3; format=json]

+       x-github-request-id: ['B0BB:248DD:3178F42:58E812F:59E76BC5']

+       x-ratelimit-limit: ['60']

+       x-ratelimit-remaining: ['37']

+       x-ratelimit-reset: ['1508341503']

+       x-runtime-rack: ['0.039710']

+       x-xss-protection: [1; mode=block]

+     status: {code: 200, message: OK}

+ - request:

+     body: null

+     headers:

+       Accept: ['*/*']

+       Accept-Encoding: ['gzip, deflate']

+       Connection: [keep-alive]

+       User-Agent: [python-requests/2.18.4]

+     method: GET

+     uri: https://api.github.com/repos/fedora-infra/something-that-does-not-exist

+   response:

+     body:

+       string: !!binary |

+         H4sIAAAAAAAAA6tWyk0tLk5MT1WyUvLLL1Fwyy/NS1HSUUrJTy7NTc0rSSzJzM+LLy3KAcpnlJQU

+         FFvp66eklqXm5BekFumlZ5ZklCbpJefn6pcZK9UCAP6TTUJNAAAA

+     headers:

+       access-control-allow-origin: ['*']

+       access-control-expose-headers: ['ETag, Link, X-GitHub-OTP, X-RateLimit-Limit,

+           X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes,

+           X-Poll-Interval']

+       content-encoding: [gzip]

+       content-security-policy: [default-src 'none']

+       content-type: [application/json; charset=utf-8]

+       date: ['Wed, 18 Oct 2017 14:57:10 GMT']

+       expect-ct: ['max-age=2592000; report-uri="https://api.github.com/_private/browser/errors"']

+       server: [GitHub.com]

+       status: [404 Not Found]

+       strict-transport-security: [max-age=31536000; includeSubdomains; preload]

+       x-content-type-options: [nosniff]

+       x-frame-options: [deny]

+       x-github-media-type: [github.v3; format=json]

+       x-github-request-id: ['5100:248DD:3178FF1:58E825A:59E76BC6']

+       x-ratelimit-limit: ['60']

+       x-ratelimit-remaining: ['36']

+       x-ratelimit-reset: ['1508341503']

+       x-runtime-rack: ['0.018622']

+       x-xss-protection: [1; mode=block]

+     status: {code: 404, message: Not Found}

+ version: 1

file modified
+16
@@ -9,6 +9,22 @@ 

  log = logging.getLogger(__name__)

  

  

+ def github_org_is_valid(username):

+     log.info("Finding github organization for {}".format(username))

+     tmpl = "https://api.github.com/users/{username}"

+     url = tmpl.format(username=username)

+     result = requests.get(url)

+     return result.ok

+ 

+ 

+ def github_repo_is_valid(username, repo):

+     log.info("Finding github repo for {} and {} ".format(repo, username))

+     tmpl = "https://api.github.com/repos/{username}/{repo}"

+     url = tmpl.format(username=username, repo=repo)

+     result = requests.get(url)

+     return result.ok

+ 

+ 

  def github_repos(token, username):

      log.info("Finding github repos for %r" % username)

      tmpl = "https://api.github.com/users/{username}/repos?per_page=100"

file modified
+6 -5
@@ -118,16 +118,17 @@ 

      """

      if not widget_config:

          return

-     config = {}

+     values = {}

      for param in widget_instance.module.get_parameters():

          val = widget_config.get(param.name)

          if not val:

              raise WidgetConfigError(

                  'You must provide a value for: %s' % param.name)

-         try:

-             config[param.name] = param.validator(val)

-         except ValueError as err:

-             raise WidgetConfigError('Invalid data provided, error: %s' % err)

+         values[param.name] = val

+     try:

+         config = widget_instance.module.validate_parameters(values)

+     except ValueError as err:

+         raise WidgetConfigError('Invalid data provided, error: %s' % err)

      # Updating in-place is not supported, it's a class property.

      cur_config = widget_instance.config or {}

      cur_config = cur_config.copy()

file modified
+18 -2
@@ -10,7 +10,6 @@ 

  from .caching import CachedFunction

  from .view import WidgetView

  

- 

  log = logging.getLogger(__name__)

  

  
@@ -100,6 +99,22 @@ 

              self.label = self.name.replace("_", " ").replace(".", ": ").title()

          self._template_environment = None

  

+     def validate_parameters(self, values):

+         """

+         Validate the parameters of a widget.

+ 

+         Args:

+             values (dict): dictionary of values associated

+                            with corresponding parameters.

+ 

+         Returns:

+             dict: validated parameter values.

+         """

+         config = {}

+         for param in self.get_parameters():

+             config[param.name] = param.validator(values[param.name])

+         return config

+ 

      def validate(self):

          """

          Ensure that the widget has the bits it needs.
@@ -273,10 +288,11 @@ 

                      "api_hub_widget", hub=instance.hub.name, idx=instance.idx),

                  "config": {},

              })

+             validated = self.validate_parameters(instance.config)

              for param in self.get_parameters():

                  if param.secret and not with_secret_config:

                      continue

-                 value = param.validator(instance.config[param.name])

+                 value = validated[param.name]

                  props["config"][param.name] = value

              if self.is_react:

                  props["component"] = self.name

@@ -34,6 +34,17 @@ 

              help="The number of tickets to display.",

          )]

  

+     def validate_parameters(self, values):

+         config = {}

+         config["org"] = \

+             validators.GithubOrganization(values["org"])

+         config["repo"] = \

+             validators.GithubRepo('/'.join([values["org"],

+                                            values["repo"]]))

+         config["display_number"] = \

+             validators.Integer(values["display_number"])

+         return config

+ 

  

  class BaseView(RootWidgetView):

  

file modified
+7 -3
@@ -10,6 +10,7 @@ 

  """

  

  from __future__ import unicode_literals

+ from hubs.utils.github import github_org_is_valid, github_repo_is_valid

  

  import flask

  import hubs.models
@@ -61,14 +62,17 @@ 

  

  def GithubOrganization(value):

      """Fails if the Github organization name does not exist."""

-     # TODO -- implement this.

+     if not github_org_is_valid(value):

+         raise ValueError('Github organization does not exist')

      return value

  

  

  def GithubRepo(value):

      """Fails if the Github repository name does not exist."""

-     # TODO -- implement this.

-     return value

+     username, repo = value.split('/')

+     if not github_repo_is_valid(username, repo):

+         raise ValueError('Github repository does not exist')

+     return repo

  

  

  def FMNContext(value):

Changed the way validation occurs: now every Widget has validate_parameters() method, which performs the validation. We can have custom validations for each Widget subclass. That was necessary because It was impossible to validate GithubRepo having only one parameter, which is reponame. Now we can use both owner and reponame to validate GithubRepo.

Fixes #351
Also, what's the matter with "The pull-request cannot be merged due to conflicts"? I don't get where the conflicts are.

Please rebase on the current develop branch, you'll see that Validators have changed a bit (they are now simple functions).

rebased onto 5fb30a12ce16d2b85a1dbac5d18a85a2221c06af

4 years ago

rebased onto 1f16def65322d5e9bd484fe9f56911275e5c1d96

4 years ago

Please rebase on the current develop branch, you'll see that Validators have changed a bit (they are now simple functions).

Not sure if rebase notifies by email, therefore leaving this comment :)

We usually don't add this type of markers.

Please add a docstring here. Also, I think that values would be a better argument name.

You can use result.ok to test for a non-error return code, that's what the requests API recommends.

I don't think that we should use a Github API token here. This token would need to be user-specific and we currently don't have a way for a user to do an Oauth2 session with GitHub.
Without the token we won't get private repos but that's not a problem for now.

rebased onto efbae2fadf8041f96d5b23f543600c7320c0d22e

4 years ago

Please describe the argument in the docstring using the format you'll see elsewhere in this file and in Hubs (Google-style).

You might as well just return result.ok.

As a general comment, also remember to run tox -e lint before pushing the PR to check for syntax and style errors.

rebased onto 8b4ad65

4 years ago

Running the tests should have created two files in hubs/tests/vcr-request-data/, please include them in your pull request.

1 new commit added

  • Added files created by testing
4 years ago

Running the tests should have created two files in hubs/tests/vcr-request-data/, please include them in your pull request.

Added mentioned files.

Pull-Request has been merged by abompard

4 years ago