refactor: name the module for the data it fetches, not for the login - #135
Conversation
otp_download_portal named the authentication step. A developer looking for bulk microdata would never find it, and the name promised a generic OTP driver. The module is not generic. It hardcodes year ids, report ids, two hosts and one sentinel. The OTP is one of five traps, not the identity of the module. It is now microdata_download_api. The summary states the artefact and the transport. The OTP, the captcha and the JavaScript-bundle API base stay in the docstring, where the context belongs. No shim. The module is unreleased under either name.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1226758362
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -1,5 +1,8 @@ | |||
| # SPDX-License-Identifier: MIT | |||
| """Download bulk microdata from a portal that gates it behind a mobile OTP. | |||
| """Download bulk statistical microdata as per-year CSV archives. | |||
There was a problem hiding this comment.
Any caller that adopted commoner_probe.otp_download_portal from the preceding feature commit now gets ModuleNotFoundError, because this rename removes that module without the compatibility shim used for every other module rename in this package. Retain an otp_download_portal.py deprecation shim that aliases the new module, and add the mapping to tests/test_module_aliases.py, so consumers do not fail merely by upgrading.
Useful? React with 👍 / 👎.
The rule this repo now follows: the module names the software, and the docstring names the deployment. geoserver names GeoServer. dspace names DSpace. nada names the World Bank's National Data Archive. This portal runs no such product. It is a bespoke build, so the name states the stack instead: a JWT-authenticated REST API behind an Angular single-page app. Three names in one day, and none of them shipped. udise named a programme. otp_download_portal named the login. microdata_download_api named the payload. Each told a developer nothing about what to implement. The docstring now leads with the reusable half. The page renders only after JavaScript runs. The build compiles the API base into one bundle, so a grep for URL literals finds nothing, and the bundle must be grepped for the variable. The auth flow is captcha, then OTP, then a bearer token. The deployment stays below it, as data: the ministry, the two hosts, the year and report ids, and the all-India sentinel of 99.
|
On the P1 — the previous path does not need a shim. Checked, not assumed. The claim is that a caller may have adopted
The name it replaced, Where the finding does apply, it was already honoured. The 21 modules The name changed again in this push, to |
1 participant
otp_download_portalnamed the authentication step. A developer looking forbulk microdata would never find it. The name also promised a generic OTP driver.
The module is not generic. It hardcodes year ids, report ids, two hosts and one
sentinel. The OTP is one of five traps, not the identity of the module.
Open question for review
This is the third name for this file, and the naming rule is still not settled
here. The rule the repo now follows: the module names the mechanism a developer
must implement; the scheme, the department and the host go in the docstring.
Two readings compete:
microdata_download_api, as inthis PR.
aspnet/aspnet_cascade— put the reusable mechanics intheir own module and keep the portal as data. The reusable half is real: the
API base is compiled into a JavaScript bundle and must be recovered by
grepping it, and auth is captcha then OTP then a JWT bearer. Both recur
across ministry single-page apps. The instance half is the year ids, the
report ids, the two hosts and the
99sentinel.Reviewers: say which, or name a better third.
Verification
1,428 passed, ruff clean. No shim, because the module is unreleased under
either name.