Repository: sec-edgar/sec-edgar
Pull request #200 (2021-04-12 22:34:05)

Adds --user-agent option to the CLI
We have added the support of supplying `--user-agent` option into the `secedgar` CLI tool. The main reasoning behind this PR is the fact that SEC mandates the user to supply sensible user agent string.

This PR builds on top of https://github.com/sec-edgar/sec-edgar/pull/198/files, of which the ability to supply `user_agent` parameter is added at the API-level.

Following this PR, the `secedgar` CLI tool can be invoked like this:

```
secedgar --user-agent='AppleWebKit/537.36 (KHTML, like Gecko)' filing  -l AAPL -t FILING_10K
```

- [ ] closes #xxxx
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] added entry to docs/whatsnew latest version

lolski: @jackmoody11 all set and ready to review!

There is this test you added, one which verifies downloading filings from the beginning of time. I had to remove it since it's taking too long time to finish: https://github.com/sec-edgar/sec-edgar/pull/200/commits/80d507e192e72c0aa4a96af59b31cf61f672a628
jackmoody11: > @jackmoody11 all set and ready to review!
> 
> 
> 
> There is this test you added, one which verifies downloading filings from the beginning of time. I had to remove it since it's taking too long time to finish: https://github.com/sec-edgar/sec-edgar/pull/200/commits/80d507e192e72c0aa4a96af59b31cf61f672a628

I wonder if it isn't being mocked properly. Regardless, I'll take a look. Thanks for all of your help on this!
lolski: >I wonder if it isn't being mocked properly. Regardless, I'll take a look. Thanks for all of your help on this!

Sounds good! In that case, I'll leave the rest to you.

Thank you for making this tool, I hope it's going to make me a mega billionaire investor some day.

Patch:
diff --git a/docs/source/whatsnew.rst b/docs/source/whatsnew.rst
index ccf3160..7c4c579 100644
--- a/docs/source/whatsnew.rst
+++ b/docs/source/whatsnew.rst
@@ -6,6 +6,8 @@ What's New
 
 New features, bug fixes, and improvements for each release.
 
+.. include:: whatsnew/v0.3.4.rst
+
 .. include:: whatsnew/v0.3.3.rst
 
 .. include:: whatsnew/v0.3.2.rst
diff --git a/docs/source/whatsnew/v0.3.4.rst b/docs/source/whatsnew/v0.3.4.rst
new file mode 100644
index 0000000..6d2db11
--- /dev/null
+++ b/docs/source/whatsnew/v0.3.4.rst
@@ -0,0 +1,15 @@
+v0.3.4
+------
+
+Highlights
+~~~~~~~~~~
+
+- Add ``user_agent`` to CLI to fix external breaking change
+
+
+Contributors
+~~~~~~~~~~~~
+
+- lolski
+- jackmoody11
+
diff --git a/secedgar/__init__.py b/secedgar/__init__.py
index 80eb7f9..fd7fa9d 100644
--- a/secedgar/__init__.py
+++ b/secedgar/__init__.py
@@ -1,1 +1,1 @@
-__version__ = '0.3.3'
+__version__ = '0.3.4-alpha'
diff --git a/secedgar/cli.py b/secedgar/cli.py
index 0b4aa8d..153919f 100644
--- a/secedgar/cli.py
+++ b/secedgar/cli.py
@@ -8,13 +8,29 @@ from secedgar.filings import DailyFilings, Filing, FilingType
 
 
 @click.group()
-def cli():
-    """Main CLI group."""
-    pass  # pragma: no cover
+@click.option('-u', '--user-agent',
+              help='Value used for HTTP header "User-Agent" for all requests.',
+              required=True,
+              type=str)
+@click.pass_context
+def cli(ctx, user_agent):
+    r"""Main CLI group.
+
+    \f
+
+    Args:
+        ctx (click.core.Context): Click context.
+        user_agent (str): User agent string to pass.
+
+    Returns:
+        None
+    """
+    ctx.ensure_object(dict)
+    ctx.obj['user_agent'] = user_agent
 
 
 def date_cleanup(date):
-    """Transforms date of form YYYYMMDD to datetime object.
+    r"""Transforms date of form YYYYMMDD to datetime object.
 
     Args:
         date (Union[str, NoneType]): Date of the form YYYYMMDD to be transformed.
@@ -43,13 +59,33 @@ def date_cleanup(date):
               help='End date for filings. Should be in the format YYYYMMDD. Defaults to today.',
               type=str)
 @click.option('-n', '--count',
-              help='Number of filings to save. Defaults to all.', type=int)
+              help='Number of filings to save. Defaults to all.',
+              type=int)
 @click.option('--directory',
               help="""Directory where files will be saved.
               Defaults to directory from which CLI is being executed.""",
               default=os.getcwd(), type=str)
-def filing(lookups, ftype, start, end, count, directory):
-    """Click command for downloading filings. Run ``secedgar filing --help`` for info."""
+@click.pass_context
+def filing(ctx, lookups, ftype, start, end, count, directory):
+    r"""Click command for downloading filings. Run ``secedgar filing --help`` for info.
+
+    \f
+
+    Args:
+        ctx (click.core.Context): Click context.
+        lookups (str): Companies and tickers to include in filing download.
+        ftype (str): String of FilingType enum.
+        start (str): Start date for filings in YYYYMMDD format.
+            Will implicitly default to first available filing.
+        end (str): End date for filings in YYYYMMDD format.
+            Will implicitly default to today.
+        count (int): Number of filings to save per ticker/company.
+        directory (str): Directory where files should be saved.
+            Defaults to current working directory.
+
+    Returns:
+        None
+    """
     # If given filing type is not valid enum, raise FilingTypeError
     try:
         ftype = FilingType[ftype]
@@ -60,7 +96,8 @@ def filing(lookups, ftype, start, end, count, directory):
                filing_type=ftype,
                start_date=date_cleanup(start),
                end_date=date_cleanup(end),
-               count=count)
+               count=count,
+               user_agent=ctx.obj['user_agent'])
     f.save(directory=directory)
 
 
@@ -70,7 +107,8 @@ def filing(lookups, ftype, start, end, count, directory):
 @click.option('--directory', help="""Directory where files will be saved.
               Defaults to directory from which CLI is being executed.""",
               default=os.getcwd(), type=str)
-def daily(date, directory):
+@click.pass_context
+def daily(ctx, date, directory):
     """Click command for downloading daily filings. Run ``secedgar daily --help`` for info."""
-    d = DailyFilings(date=date_cleanup(date))
+    d = DailyFilings(date=date_cleanup(date), user_agent=ctx.obj['user_agent'])
     d.save(directory=directory)
diff --git a/secedgar/filings/daily.py b/secedgar/filings/daily.py
index 158784e..1e060bf 100644
--- a/secedgar/filings/daily.py
+++ b/secedgar/filings/daily.py
@@ -17,6 +17,7 @@ class DailyFilings(IndexFilings):
             The ``FilingEntry`` object exposes 6 variables which can be
             used to filter which filings to keep. These are "cik", "company_name",
             "form_type", "date_filed", "file_name", and "path".
+        kwargs: Keyword arguments to pass to ``secedgar.filings._index.IndexFilings``.
 
     Using ``entry_filter``
     ----------------------
@@ -49,8 +50,8 @@ class DailyFilings(IndexFilings):
 
     """
 
-    def __init__(self, date, client=None, entry_filter=lambda _: True):
-        super().__init__(client=client, entry_filter=entry_filter)
+    def __init__(self, date, client=None, entry_filter=lambda _: True, **kwargs):
+        super().__init__(client=client, entry_filter=entry_filter, **kwargs)
         if not isinstance(date, datetime.date):
             raise TypeError(
                 "Date must be given as datetime.date object. Was given type {type}.".format(


Test patch:
diff --git a/secedgar/tests/conftest.py b/secedgar/tests/conftest.py
index ac4fd89..f60df44 100644
--- a/secedgar/tests/conftest.py
+++ b/secedgar/tests/conftest.py
@@ -1,5 +1,6 @@
 import pytest
 import requests
+from secedgar.cik_lookup import CIKLookup
 from secedgar.client import NetworkClient
 from secedgar.filings import MasterFilings
 from secedgar.tests.utils import AsyncMockResponse, MockResponse, datapath
@@ -66,3 +67,17 @@ def mock_filing_data(monkeysession):
 @pytest.fixture(scope="session")
 def tmp_data_directory(tmpdir_factory):
     return str(tmpdir_factory.mktemp("tmp_data"))
+
+
+@pytest.fixture(scope="session")
+def mock_cik_validator_get_multiple_ciks(monkeysession):
+    """Mocks response for getting a single CIK."""
+    monkeysession.setattr(CIKLookup, "get_ciks",
+                          lambda *args: {"aapl": "0000320193", "msft": "1234", "amzn": "5678"})
+
+
+@pytest.fixture(scope="session")
+def mock_single_cik_filing(monkeysession):
+    """Returns mock response of filinghrefs for getting filing URLs."""
+    monkeysession.setattr(NetworkClient, "get_response",
+                          MockResponse(datapath_args=["filings", "aapl_10q_filings.xml"]))
diff --git a/secedgar/tests/filings/test_filings.py b/secedgar/tests/filings/test_filings.py
index 7989996..a6c7cd7 100644
--- a/secedgar/tests/filings/test_filings.py
+++ b/secedgar/tests/filings/test_filings.py
@@ -16,13 +16,6 @@ def mock_cik_validator_get_single_cik(monkeypatch):
                         lambda *args, **kwargs: {"aapl": "0000320193"})
 
 
-@pytest.fixture(scope="module")
-def mock_cik_validator_get_multiple_ciks(monkeymodule):
-    """Mocks response for getting a single CIK."""
-    monkeymodule.setattr(CIKLookup, "get_ciks",
-                         lambda *args: {"aapl": "0000320193", "msft": "1234", "amzn": "5678"})
-
-
 @pytest.fixture(scope="module")
 def mock_single_cik_not_found(monkeymodule):
     """NetworkClient get_response method will return html with CIK not found message."""
@@ -30,13 +23,6 @@ def mock_single_cik_not_found(monkeymodule):
                          MockResponse(datapath_args=["CIK", "cik_not_found.html"]))
 
 
-@pytest.fixture(scope="module")
-def mock_single_cik_filing(monkeymodule):
-    """Returns mock response of filinghrefs for getting filing URLs."""
-    monkeymodule.setattr(NetworkClient, "get_response",
-                         MockResponse(datapath_args=["filings", "aapl_10q_filings.xml"]))
-
-
 class MockSingleCIKFilingLimitedResponses:
     def __init__(self, num_responses):
         self._called_count = 0
diff --git a/secedgar/tests/test_cli.py b/secedgar/tests/test_cli.py
index 48daf80..f6871be 100644
--- a/secedgar/tests/test_cli.py
+++ b/secedgar/tests/test_cli.py
@@ -1,59 +1,106 @@
+import os
+
 import pytest
 from click.testing import CliRunner
-from secedgar.cli import daily, filing
+from secedgar.cli import cli
 from secedgar.exceptions import FilingTypeError
+# Borrow mocks without including in conftest
+from secedgar.tests.filings.test_daily import mock_daily_idx_file  # noqa: F401
+from secedgar.tests.filings.test_daily import \
+    mock_daily_quarter_directory  # noqa: F401
 
 
-def run_cli_command(cli, user_input, directory, catch_exceptions=False):
+def run_cli_command(cli_instance,
+                    user_input,
+                    directory=None,
+                    user_agent="'My User Agent (email@example.com)'",
+                    catch_exceptions=False):
     runner = CliRunner()
-    user_input = user_input + " --directory {}".format(directory)
-    return runner.invoke(cli, user_input, catch_exceptions=catch_exceptions)
+    user_input = user_input.split()
+    user_input = ['--user-agent', user_agent] + user_input + ['--directory', directory]
+    return runner.invoke(cli_instance, user_input, catch_exceptions=catch_exceptions)
 
 
-def check_bad_inputs(cli, user_input, expected_exception, directory):
+def check_bad_inputs(cli_instance,
+                     user_input,
+                     expected_exception,
+                     directory,
+                     user_agent="'My User Agent (email@example.com)'"):
     # SystemExit does not raise exception by runner
     if expected_exception is SystemExit:
-        result = run_cli_command(cli, user_input, directory)
+        result = run_cli_command(cli_instance=cli_instance,
+                                 user_input=user_input,
+                                 directory=directory,
+                                 user_agent=user_agent)
         assert result.exit_code != 0
     else:
         with pytest.raises(expected_exception):
-            run_cli_command(cli, user_input, directory)
-
+            run_cli_command(cli_instance=cli_instance,
+                            user_input=user_input,
+                            directory=directory,
+                            user_agent=user_agent)
 
-class TestCLIFiling:
 
+class TestCLI:
     @pytest.mark.parametrize(
         "user_input,expected_exception",
         [
-            ("-l aapl msft Facebook", SystemExit),  # missing filing type
-            ("-l aapl -t null", FilingTypeError),  # unrecognized filing type
-            ("-l aapl -t FILING_10Q -n abc", SystemExit),  # count is not int
-            ("-l aapl -t FILING_10Q -n 0", ValueError)  # no filings available if 0 picked
+            ("filing -l aapl msft Facebook", SystemExit),  # missing filing type
+            ("filing -l aapl -t null", FilingTypeError),  # unrecognized filing type
+            ("filing -l aapl -t FILING_10Q -n abc", SystemExit),  # count is not int
+            ("filing -l aapl -t FILING_10Q -n 0",
+             ValueError),  # no filings available if 0 picked
         ]
     )
-    def test_filing_bad_inputs(self, user_input, expected_exception, tmp_data_directory):
-        check_bad_inputs(filing, user_input, expected_exception, tmp_data_directory)
+    def test_cli_filing_bad_inputs(self, user_input, expected_exception, tmp_data_directory):
+        check_bad_inputs(cli,
+                         user_input=user_input,
+                         expected_exception=expected_exception,
+                         directory=tmp_data_directory)
 
     @pytest.mark.parametrize(
-        "user_input",
+        "user_input,count",
         [
-            "-l aapl msft fb FILING_10Q",
-            "-l aapl msft fb FILING_10Q -n 10",
-            "-l aapl msft fb FILING_10Q -n 1"
+            ("filing -l aapl -l msft -l amzn -t FILING_10Q -n {}", 10),
         ]
     )
-    def test_multiple_companies_input(self, user_input, tmp_data_directory):
-        pass
-
-
-class TestCLIDaily:
+    def test_cli_filing_multiple_companies_input(
+            self,
+            user_input,
+            count,
+            tmp_data_directory,
+            mock_cik_validator_get_multiple_ciks,
+            mock_single_cik_filing,
+            mock_filing_response):
+        if count is not None:
+            user_input = user_input.format(count)
+        result = run_cli_command(cli, user_input, tmp_data_directory)
+        assert result.exit_code == 0
+        txt_files = [f for *_, files in os.walk(tmp_data_directory) for f in files]
+        if count is None:
+            assert len(txt_files) == 3
+        else:
+            assert len(txt_files) == 3 * count
 
     @pytest.mark.parametrize(
         "user_input,expected_exception",
         [
-            ("", SystemExit),
-            ("-d 2020", ValueError)
+            ("daily", SystemExit),
+            ("daily -d 2020", ValueError)
+        ]
+    )
+    def test_cli_daily_bad_inputs(self, user_input, expected_exception, tmp_data_directory):
+        check_bad_inputs(cli,
+                         user_input=user_input,
+                         expected_exception=expected_exception,
+                         directory=tmp_data_directory)
+
+    @pytest.mark.parametrize(
+        "user_input",
+        [
+            "daily -d 20201113",
+            "filing -l aapl -l msft -l amzn -t FILING_10Q -n 10"
         ]
     )
-    def test_daily_bad_inputs(self, user_input, expected_exception, tmp_data_directory):
-        check_bad_inputs(daily, user_input, expected_exception, tmp_data_directory)
+    def test_cli_requires_user_agent(self, user_input, tmp_data_directory):
+        check_bad_inputs(cli, user_input, SystemExit, tmp_data_directory, user_agent=None)