0
\$\begingroup\$

I am trying to implement a parent WebContent class and child classes to grab HTTP responses from the actual website entities. There are highlevel codes below and I am wondering what are people's perspectives in terms of the neatest way to implement this in a OOP manner.

import requests
from onelogin.api.client import OneLoginClient


class WebContent(object):

    def __init__(self, client_id, client_secret, login, password, sub_domain, app_id, app_url):

        self.client_id = client_id
        self.client_secret = client_secret
        self.app_id = app_id
        self.app_url = app_url
        self.login = login
        self.password = password
        self.sub_domain = sub_domain

    def _login(self):
        client = OneLoginClient(self.client_id, self.client_secret)
        saml = client.get_saml_assertion(self.login,
                                         self.password,
                                         self.app_id,
                                         self.sub_domain)
        saml_data = saml.saml_response

        session = requests.Session()
        saml_payload = {'SAMLResponse': saml_data}
        session.post("{}/sso/response".format(self.app_url), saml_payload)
        return session

    def get_content(self, endpoint, time_out=30):
        if endpoint:
            session = self._login()
            result = session.get(endpoint, timeout=time_out)
            session.close()
            return result.content


class WebMarketingContent(WebContent):
    def get_endpoint(self, entity_id):
        base_url = "{app_url}/{entity_id}?{query_params}"
        params = '&entity_id={}'.format(entity_id)
        return base_url.format(app_url=self.app_url, query_params=params)


class WebEducationContent(WebContent):
    def get_endpoint(self, entity_id):
        base_url = "{app_url}/category/adhoc_element/{entity_id}?{query_params}"
        params = '&entity_id={}'.format(entity_id)
        return base_url.format(app_url=self.app_url, query_params=params)


if __name__ == '__main__':
    web_marketing_content = WebMarketingContent('client_id',
                                                'client_secret',
                                                'email',
                                                'password',
                                                'sub_domain',
                                                'app_id',
                                                'app_url')

    endpoint = web_marketing_content.get_endpoint(123)
    result = web_marketing_content.get_content(endpoint)
\$\endgroup\$
0
\$\begingroup\$

Some suggestions:

  1. if __name__ == '__main__': is usually followed by a call to a main method. The main method then parses arguments using for example argparse to inject all the things which should not be hardcoded in the application - definitely passwords, but in your case probably all of the parameters to WebMarketingContent. This makes the code reusable and scriptable.
  2. WebEducationContent is not used anywhere, so it should be removed.
  3. Take advantage of static analysis and formatting using black, flake8 and mypy with a strict configuration to improve the overall quality.
  4. You shouldn't need to get the endpoint before getting the content. web_marketing_content.get_content(123) should itself work out the endpoint and request it.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.