uranusjr commented on code in PR #34060:
URL: https://github.com/apache/airflow/pull/34060#discussion_r1314517696
##########
airflow/www/extensions/init_views.py:
##########
@@ -253,15 +254,15 @@ def _handle_method_not_allowed(ex):
else:
return views.method_not_allowed(ex)
- with open(path.join(ROOT_APP_DIR, "api_connexion", "openapi", "v1.yaml"))
as f:
+ with ROOT_APP_DIR.joinpath("api_connexion", "openapi", "v1.yaml").open()
as f:
specification = safe_load(f)
api_bp = FlaskApi(
specification=specification,
resolver=_LazyResolver(),
base_path=base_path,
options={
"swagger_ui": conf.getboolean("webserver", "enable_swagger_ui",
fallback=True),
- "swagger_path": path.join(ROOT_APP_DIR, "www", "static", "dist",
"swagger-ui"),
+ "swagger_path": os.fspath(ROOT_APP_DIR / "www" / "static" / "dist"
/ "swagger-ui"),
Review Comment:
```suggestion
"swagger_path": os.fspath(ROOT_APP_DIR /
"www/static/dist/swagger-ui"),
```
The performance difference is not insignificant on its own due to the fact
that every `/` needs to go through a magic method (it’s not much in the grand
scheme of things of course).
```python
py -m timeit -s 'from pathlib import Path; p = Path.cwd()' 'p / "www" /
"static" / "dist" / "swagger-ui"'
50000 loops, best of 5: 5.65 usec per loop
uranusjr@Stuuuu airflow $ py -m timeit -s 'from pathlib import Path; p =
Path.cwd()' 'p / "www/static/dist/swagger-ui"'
200000 loops, best of 5: 1.37 usec per loop
```
Alternatively `joinpath` is also much better for this.
##########
airflow/www/extensions/init_views.py:
##########
@@ -253,15 +254,15 @@ def _handle_method_not_allowed(ex):
else:
return views.method_not_allowed(ex)
- with open(path.join(ROOT_APP_DIR, "api_connexion", "openapi", "v1.yaml"))
as f:
+ with ROOT_APP_DIR.joinpath("api_connexion", "openapi", "v1.yaml").open()
as f:
specification = safe_load(f)
api_bp = FlaskApi(
specification=specification,
resolver=_LazyResolver(),
base_path=base_path,
options={
"swagger_ui": conf.getboolean("webserver", "enable_swagger_ui",
fallback=True),
- "swagger_path": path.join(ROOT_APP_DIR, "www", "static", "dist",
"swagger-ui"),
+ "swagger_path": os.fspath(ROOT_APP_DIR / "www" / "static" / "dist"
/ "swagger-ui"),
Review Comment:
```suggestion
"swagger_path": os.fspath(ROOT_APP_DIR /
"www/static/dist/swagger-ui"),
```
The performance difference is not insignificant on its own due to the fact
that every `/` needs to go through a magic method (it’s not much in the grand
scheme of things of course).
```python
py -m timeit -s 'from pathlib import Path; p = Path.cwd()' 'p / "www" /
"static" / "dist" / "swagger-ui"'
50000 loops, best of 5: 5.65 usec per loop
uranusjr@Stuuuu airflow $ py -m timeit -s 'from pathlib import Path; p =
Path.cwd()' 'p / "www/static/dist/swagger-ui"'
200000 loops, best of 5: 1.37 usec per loop
```
Alternatively `joinpath` is also much better for this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
