Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement optional url prefix the Django way #15080

Merged

Conversation

chrismeyersfsu
Copy link
Member

@chrismeyersfsu chrismeyersfsu commented Apr 8, 2024

SUMMARY

Performance investigation notes.
As I suspected, an implementation that constructs the urls patterns foreach request is noticeable.

The bottom set of inclusive times are when visiting /api/v2/hosts/ vs. the top set are when visiting /awx/v2/<prefix>/hosts/. The exclusive entry is isolated to just get_urlpatterns(prefix=settings.OPTIONAL_API_URLPATTERN_PREFIX)

-rw-r--r-- 1 awx  root   55 Apr  9 12:52 0.000253-seconds-exclusive-8d3c0d32-8c04-4f30-b555-1003918494ca.time
-rw-r--r-- 1 awx  root   55 Apr  9 12:53 0.000363-seconds-exclusive-695d75fa-78c6-452a-a75f-635cd3731445.time
-rw-r--r-- 1 awx  root   54 Apr  9 12:53 0.00043-seconds-exclusive-3357476d-9dfb-4e68-a502-7a9536b820d5.time
-rw-r--r-- 1 awx  root   55 Apr  9 12:52 0.000513-seconds-inclusive-3b9e3ba2-f744-4c8f-b2bb-b971398a3db4.time
-rw-r--r-- 1 awx  root   55 Apr  9 12:53 0.000753-seconds-inclusive-9f25b670-b58b-471d-aab5-77f09e5be12a.time
-rw-r--r-- 1 awx  root   55 Apr  9 12:53 0.000863-seconds-inclusive-de8d5fb6-1693-41b2-bdc3-1ed58ff219bb.time
-rw-r--r-- 1 awx  root   54 Apr  9 12:50 1.1e-05-seconds-inclusive-af3f6f83-b2d4-4f52-9ea2-2957357e8a85.time
-rw-r--r-- 1 awx  root   54 Apr  9 12:41 1.2e-05-seconds-inclusive-419adff1-1900-4236-bd13-bcdb16ecddb6.time
-rw-r--r-- 1 awx  root   54 Apr  9 12:41 1.3e-05-seconds-inclusive-a396c7bb-b1cf-4710-be53-adac84f47c08.time
-rw-r--r-- 1 awx  root   54 Apr  9 12:50 1.3e-05-seconds-inclusive-cf462f74-339f-4717-8a80-af892499c1a6.time
-rw-r--r-- 1 awx  root   54 Apr  9 12:41 1.4e-05-seconds-inclusive-1bbe3da6-ff8d-4c0f-a705-e80b28201c47.time
-rw-r--r-- 1 awx  root   54 Apr  9 12:41 1.5e-05-seconds-inclusive-021aa202-2461-4a57-ad92-3fb9cfd0661f.time

Done:

  • cache get_urlpatterns(prefix=settings.OPTIONAL_API_URLPATTERN_PREFIX) results
  • maybe call get_urlpatterns(prefix=settings.OPTIONAL_API_URLPATTERN_PREFIX) in some sort of init process so we don't pay the penalty on first request. Realistically though, the penalty here isn't that expensive if we do have to pay it 1 time per new uwsgi process. decided this isn't worth it.
ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • API
AWX VERSION
devel
ADDITIONAL INFORMATION

@chrismeyersfsu chrismeyersfsu force-pushed the AAP-22562-service-index-urls branch 7 times, most recently from 39c0c1a to 8f46f39 Compare April 9, 2024 19:17
@chrismeyersfsu chrismeyersfsu marked this pull request as ready for review April 9, 2024 19:27
@chrismeyersfsu chrismeyersfsu changed the title Aap 22562 service index urls Implement optional url prefix the Django way Apr 9, 2024
* Before, the optional url prefix feature required calling our
  versioning version of reverse(). This worked _ok_ until we added more
  and more urls from 3rd party apps. Those 3rd party apps do not call
  our reverse(), writefully so.
* This implementation looks at the incoming request path. If it includes
  the special optional prefix url, then we register ALL the urls WITH
  the optional url prefix.
  If the incoming request path does NOT contain the options url prefix
  then we register ALL the urls WITHOUT the optional url prefix.
* Before this, we were registering BOTH sets of urls and then reverse()
  + the request as context to decide which url.
@chrismeyersfsu chrismeyersfsu merged commit 0645d34 into ansible:devel Apr 10, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants