Overhaul locations demo and apply various fixes#17
Overhaul locations demo and apply various fixes#17jaleman-vdr-wikimedia merged 3 commits intowikimedia-enterprise:mainfrom
Conversation
This commit includes the following changes: - The locations demo was completely refactored. It now uses HTTPX, is async, and works. Previously, demo was only partially functional. - Requirements.txt was updated to actually include all requirements. - get.py from word_cloud demo was not functional. A simple code line placement was all that it took to fix. - Readme was also updated to properly reflect script invocation. - Removed personal comment annotations from sdk_logs - Corrected Readme for elections demo to ensure the proper run syntax is shown. - Small refactor change in get script for elections demo to ensure it actually works.
This commit adds installation dependencies for the Pylint workflow.
| @@ -18,6 +18,17 @@ jobs: | |||
| run: | | |||
| python -m pip install --upgrade pip | |||
| pip install pylint | |||
There was a problem hiding this comment.
Why isn't this using requirements.txt?
There was a problem hiding this comment.
Unfortunately, I'm not quite sure. This pylint.yml file was always there, and I assumed this was the structure the team wanted. Due to that, I worked around that assumption adding required dependencies.
| environment variables to the WME login endpoint. The resulting | ||
| 'ACCESS_TOKEN' is stored in the global `ACCESS_TOKEN` variable for use | ||
| in subsequent API calls. | ||
| yield |
There was a problem hiding this comment.
Add a comment explainig why we yield here.
| app = FastAPI(lifespan=lifespan) | ||
|
|
||
| # --- Template and Static File Setup --- | ||
| templates = Jinja2Templates(directory="templates") | ||
| app.mount("/static", StaticFiles(directory="static"), name="static") |
There was a problem hiding this comment.
Can we put this under if __name__ == "__main__":, at the bottom?
There was a problem hiding this comment.
Moving it would break the graceful shutdown and the httpx client would never be closed. Given that uvicorn.run starts the server and mantains the script in that line until termination, any code after that won't be reached.
| message on failure. | ||
| """ | ||
| current_access_token = state.access_token | ||
| url = f"https://api.enterprise.wikimedia.com/v2/structured-contents/{title}" |
There was a problem hiding this comment.
Very confusing that this is not using the SDK. I can see that's not a change you made, just wanted to point it out.
This commit includes a comment to explain a yield as per feedback.
jaleman-vdr-wikimedia
merged commit 3aaa921
into
wikimedia-enterprise:main
2 participants
This PR aims to add the following
The locations demo was completely refactored. It now uses HTTPX, is async, and works. Previously, demo was only partially functional.
Requirements.txt was updated to actually include all requirements.
get.py from word_cloud demo was not functional. A simple code line placement was all that it took to fix.
Readme was also updated to properly reflect script invocation.
Removed personal comment annotations from sdk_logs
Corrected Readme for elections demo to ensure the proper run syntax is shown.
Small refactor change in get script for elections demo to ensure it actually works.
Add dependencies for Pylint's workflow correct execution.