open-contracting / credere-backend

A tool that facilitates the participation of Micro, Small, and Medium businesses (MSMEs) in the Colombian public procurement market.
https://credere.readthedocs.io
BSD 3-Clause "New" or "Revised" License
5 stars 0 forks source link

perf: Reuse httpx.Client, use orjson #314

Closed jpmckinney closed 1 month ago

jpmckinney commented 1 month ago

https://www.python-httpx.org/advanced/clients/

coveralls commented 1 month ago

Pull Request Test Coverage Report for Build 10335280865

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
app/routers/guest/applications.py 1 2 50.0%
app/mail.py 16 18 88.89%
app/sources/init.py 1 4 25.0%
app/sources/colombia.py 13 16 81.25%
app/commands.py 26 32 81.25%
<!-- Total: 87 102 85.29% -->
Files with Coverage Reduction New Missed Lines %
app/routers/guest/applications.py 1 84.47%
app/util.py 1 96.74%
app/models.py 2 99.11%
app/sources/colombia.py 3 69.05%
<!-- Total: 7 -->
Totals Coverage Status
Change from base Build 10334100384: 0.05%
Covered Lines: 1967
Relevant Lines: 2283

💛 - Coveralls
jpmckinney commented 1 month ago

Okay, done reviewing code in the fetch_all_awards_from_period path. I don't see much that would cause the server to exhaust resources. Might need to monitor actively if there's a subtle leak – tail /var/log/apache2/error.log to see if those errors occur again, and maybe installing py-spy like on the kingfisher server to monitor the live process.

In terms of leaks, SQLAlchemy seems to have a LRU Cache of only 500 items, so it's probably not DB caching: https://docs.sqlalchemy.org/en/20/core/connections.html#configuration

yolile commented 1 month ago

Mmm I'm getting this error when trying to deploy the back end:

Process SpawnProcess-1:
Traceback (most recent call last):
  File "/usr/lib/python3.11/multiprocessing/process.py", line 314, in _bootstrap
    self.run()
  File "/usr/lib/python3.11/multiprocessing/process.py", line 108, in run
    self._target(*self._args, **self._kwargs)
  File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/uvicorn/_subprocess.py", line 76, in subprocess_started
    target(sockets=sockets)
  File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/uvicorn/server.py", line 61, in run
    return asyncio.run(self.serve(sockets=sockets))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/base_events.py", line 654, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/uvicorn/server.py", line 68, in serve
    config.load()
  File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/uvicorn/config.py", line 473, in load
    self.loaded_app = import_from_string(self.app)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/uvicorn/importer.py", line 21, in import_from_string
    module = importlib.import_module(module_str)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1204, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1176, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1147, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 690, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 940, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/home/yolile/development/credere-backend/app/main.py", line 4, in <module>
    from app.routers import applications, downloads, guest, lenders, statistics, users
  File "/home/yolile/development/credere-backend/app/routers/applications.py", line 11, in <module>
    from app import dependencies, models, parsers, serializers, util
  File "/home/yolile/development/credere-backend/app/dependencies.py", line 8, in <module>
    from app import auth, models, parsers, util
  File "/home/yolile/development/credere-backend/app/util.py", line 21, in <module>
    from app.sources import colombia as data_access
  File "/home/yolile/development/credere-backend/app/sources/__init__.py", line 9, in <module>
    client = httpx.Client(transport=httpx.HTTPTransport(retries=3, verify=False), timeout=60)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/httpx/_client.py", line 646, in __init__
    super().__init__(
  File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/httpx/_client.py", line 181, in __init__
    self._params = QueryParams(params)
                   ^^^^^^^^^^^^^^^^^^^
  File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/httpx/_urls.py", line 427, in __init__
    self._dict = parse_qs(value, keep_blank_values=True)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/urllib/parse.py", line 718, in parse_qs
    pairs = parse_qsl(qs, keep_blank_values, strict_parsing,
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/urllib/parse.py", line 768, in parse_qsl
    qs = bytes(qs)
         ^^^^^^^^^
TypeError: cannot convert 'NoneType' object to bytes

And a similar one when trying to run commands:

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/home/yolile/development/credere-backend/app/commands.py", line 12, in <module>
    from app import mail, models, util
  File "/home/yolile/development/credere-backend/app/util.py", line 21, in <module>
    from app.sources import colombia as data_access
  File "/home/yolile/development/credere-backend/app/sources/__init__.py", line 9, in <module>
    client = httpx.Client(transport=httpx.HTTPTransport(retries=3, verify=False), timeout=60)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/httpx/_client.py", line 646, in __init__
    super().__init__(
  File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/httpx/_client.py", line 181, in __init__
    self._params = QueryParams(params)
                   ^^^^^^^^^^^^^^^^^^^
  File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/httpx/_urls.py", line 427, in __init__
    self._dict = parse_qs(value, keep_blank_values=True)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/urllib/parse.py", line 718, in parse_qs
    pairs = parse_qsl(qs, keep_blank_values, strict_parsing,
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/urllib/parse.py", line 768, in parse_qsl
    qs = bytes(qs)
         ^^^^^^^^^
TypeError: cannot convert 'NoneType' object to bytes
jpmckinney commented 1 month ago

I’ll have a look on Monday. This PR doesn’t fix any bugs, just does some cleanup and small performance improvements.

On Saturday, August 10, 2024, Yohanna Lisnichuk @.***> wrote:

Mmm I'm getting this error when trying to deploy the back end:

Process SpawnProcess-1: Traceback (most recent call last): File "/usr/lib/python3.11/multiprocessing/process.py", line 314, in _bootstrap self.run() File "/usr/lib/python3.11/multiprocessing/process.py", line 108, in run self._target(*self._args, **self._kwargs) File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/uvicorn/_subprocess.py", line 76, in subprocess_started target(sockets=sockets) File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/uvicorn/server.py", line 61, in run return asyncio.run(self.serve(sockets=sockets)) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/asyncio/runners.py", line 190, in run return runner.run(main) ^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/asyncio/runners.py", line 118, in run return self._loop.run_until_complete(task) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/asyncio/base_events.py", line 654, in run_until_complete return future.result() ^^^^^^^^^^^^^^^ File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/uvicorn/server.py", line 68, in serve config.load() File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/uvicorn/config.py", line 473, in load self.loaded_app = import_from_string(self.app) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/uvicorn/importer.py", line 21, in import_from_string module = importlib.import_module(module_str) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/importlib/init.py", line 126, in import_module return _bootstrap._gcd_import(name[level:], package, level) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "", line 1204, in _gcd_import File "", line 1176, in _find_and_load File "", line 1147, in _find_and_load_unlocked File "", line 690, in _load_unlocked File "", line 940, in exec_module File "", line 241, in _call_with_frames_removed File "/home/yolile/development/credere-backend/app/main.py", line 4, in from app.routers import applications, downloads, guest, lenders, statistics, users File "/home/yolile/development/credere-backend/app/routers/applications.py", line 11, in from app import dependencies, models, parsers, serializers, util File "/home/yolile/development/credere-backend/app/dependencies.py", line 8, in from app import auth, models, parsers, util File "/home/yolile/development/credere-backend/app/util.py", line 21, in from app.sources import colombia as data_access File "/home/yolile/development/credere-backend/app/sources/init.py", line 9, in client = httpx.Client(transport=httpx.HTTPTransport(retries=3, verify=False), timeout=60) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/httpx/_client.py", line 646, in init super().init( File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/httpx/_client.py", line 181, in init self._params = QueryParams(params) ^^^^^^^^^^^^^^^^^^^ File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/httpx/_urls.py", line 427, in init self._dict = parse_qs(value, keep_blank_values=True) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/urllib/parse.py", line 718, in parse_qs pairs = parse_qsl(qs, keep_blank_values, strict_parsing, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/urllib/parse.py", line 768, in parse_qsl qs = bytes(qs) ^^^^^^^^^ TypeError: cannot convert 'NoneType' object to bytes

And a similar one when trying to run commands:

Traceback (most recent call last): File "", line 198, in _run_module_as_main File "", line 88, in _run_code File "/home/yolile/development/credere-backend/app/commands.py", line 12, in from app import mail, models, util File "/home/yolile/development/credere-backend/app/util.py", line 21, in from app.sources import colombia as data_access File "/home/yolile/development/credere-backend/app/sources/init.py", line 9, in client = httpx.Client(transport=httpx.HTTPTransport(retries=3, verify=False), timeout=60) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/httpx/_client.py", line 646, in init super().init( File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/httpx/_client.py", line 181, in init self._params = QueryParams(params) ^^^^^^^^^^^^^^^^^^^ File "/home/yolile/development/credere-backend/.ve/lib/python3.11/site-packages/httpx/_urls.py", line 427, in init self._dict = parse_qs(value, keep_blank_values=True) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/urllib/parse.py", line 718, in parse_qs pairs = parse_qsl(qs, keep_blank_values, strict_parsing, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/urllib/parse.py", line 768, in parse_qsl qs = bytes(qs) ^^^^^^^^^ TypeError: cannot convert 'NoneType' object to bytes

— Reply to this email directly, view it on GitHub https://github.com/open-contracting/credere-backend/pull/314#issuecomment-2282251770, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAGOX2OFMAZ7T6B77LJKM3ZQZRTDAVCNFSM6AAAAABMJOY6R2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOBSGI2TCNZXGA . You are receiving this because you authored the thread.Message ID: @.***>

--

James McKinney

Head of Technology

+1-514-247-0223 | @mckinneyjames | timezone: EST

🚀 Read our 2023 Annual Report https://www.open-contracting.org/annual-report-2023 for 11 stories of transformational change making public procurement more accountable, inclusive and sustainable.

www.open-contracting.org | follow us @opencontracting

jpmckinney commented 1 month ago

Maybe we need to restore the params={} keyword argument from the original code (which didn’t make any sense, but this error doesn’t make sense either).

jpmckinney commented 1 month ago

It's a Python bug. Upgrade to 3.11.9. https://docs.python.org/release/3.11.9/whatsnew/changelog.html#library

gh-116764: Restore support of None and other false values in urllib.parse functions parse_qs() and parse_qsl(). Also, they now raise a TypeError for non-zero integers and non-empty sequences.

Image has 3.11.9 already:

$ docker compose --progress=quiet run --rm --name credere-backend-shell cron python --version
Python 3.11.9
yolile commented 1 month ago

Thanks, that was the issue indeed. However, I now get:

Traceback (most recent call last):

  File "<frozen runpy>", line 198, in _run_module_as_main

  File "<frozen runpy>", line 88, in _run_code

  File "/home/yolile/development/credere-backend/app/commands.py", line 459, in <module>
    app()

  File "/home/yolile/development/credere-backend/app/commands.py", line 157, in fetch_all_awards_from_period
    _get_awards_from_data_source(from_date, get_db, until_date)

  File "/home/yolile/development/credere-backend/app/commands.py", line 124, in _get_awards_from_data_source
    _create_complete_application(entry, db_provider)

  File "/home/yolile/development/credere-backend/app/commands.py", line 30, in _create_complete_application
    award = util.create_award_from_data_source(session, award_entry)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/home/yolile/development/credere-backend/app/util.py", line 172, in create_award_from_data_source
    data = data_access.get_award(entry, borrower_id, previous)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/home/yolile/development/credere-backend/app/sources/colombia.py", line 80, in get_award
    contract_response_json, contract_url = _get_remote_contract(proceso_de_compra, proveedor_adjudicado, previous)
                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/home/yolile/development/credere-backend/app/sources/colombia.py", line 60, in _get_remote_contract
    return util.loads(sources.make_request_with_retry(contract_url, HEADERS).json()), contract_url
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "/home/yolile/development/credere-backend/app/util.py", line 42, in loads
    return orjson.loads(response.text)
                        ^^^^^^^^^^^^^

AttributeError: 'list' object has no attribute 'text'

So I guess we need

return util.loads(sources.make_request_with_retry(contract_url, HEADERS)), contract_url

instead of

return util.loads(sources.make_request_with_retry(contract_url, HEADERS).json()), contract_url

jpmckinney commented 1 month ago

So I guess we need

return util.loads(sources.make_request_with_retry(contract_url, HEADERS)), contract_url

instead of

return util.loads(sources.make_request_with_retry(contract_url, HEADERS).json()), contract_url

Yeah, I amended this commit but I guess it got readded. https://github.com/open-contracting/credere-backend/pull/314/commits/fa8dad3811be17179a4fb28ce9ca8b9675fa7fde