Added ability to use suffixes with a controller that has been already registered without suffixes.

[ebensonwwg]

No description provided.

[codecov-commenter]

Codecov Report

Merging #23 into master will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master       #23   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines           40        53   +13     
=========================================
+ Hits            40        53   +13     

Impacted Files	Coverage Δ
falcon_apispec/falcon_plugin.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5e665e...346f65b. Read the comment docs.

[alysivji]

Thanks for making this PR!

It's July 4th long weekend, won't be able to review until next weekend. Appreciate your understanding.

[ebensonwwg]

Hope you enjoyed the long weekend. I appreciate your update and look forward to your comments and merge this weekend.

[alysivji]

Unless people read this README, they are not going to know about this convention.

Is there a better way to register resources? Would prefer to do it once versus n+1 times. Also, is there is a reason it's n+1?

[ebensonwwg]

Hi, Unfortunately not, due to how the ApiSpec plugin works. It calls each resource once, meaning if you use a controller twice only one path will be chosen.

[ebensonwwg]

@alysivji Any updates?

[alysivji]

Calling something n+1 times is a sign of bad design. What if y'all add a new suffix but forgot to add the line? Having a manual process is a recipe for disaster.

There has to be a way of fixing it I would need to look into how the path_helper works... why can't we loop n+1 times inside there to get the information you need?

I have commitments until August 4th. I can definitely help out after then.

[ebensonwwg]

@alysivji Status?

[ebensonwwg]

I will follow up on Monday with my coworker. Eric Benson

________________________________ From: Aly Sivji <notifications@github.com> Sent: Saturday, July 11, 2020 6:35:46 PM To: alysivji/falcon-apispec <falcon-apispec@noreply.github.com> Cc: Benson, Eric <Eric.Benson@grainger.com>; Author <author@noreply.github.com> Subject: Re: [alysivji/falcon-apispec] Added ability to use suffixes with a controller that has been already registered without suffixes. (#23) @alysivji requested changes on this pull request.

________________________________ In README.md<https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Falysivji%2Ffalcon-apispec%2Fpull%2F23%23discussion_r453245764&data=02%7C01%7Ceric.benson%40grainger.com%7Ce336f8a04ae542737b3408d825f32349%7C48d1dcb6bccc4365ac7fb937a7f7fd71%7C0%7C0%7C637301073506730807&sdata=QKnkRL6lrX0IuyufHaU8ipVqc2HSdLZlBf%2FY2o0oJvY%3D&reserved=0>:

+# should be passed twice so the suffix is registered

+# path should be called n + 1 times where n is the number of suffixes Unless people read this README, they are not going to know about this convention. Is there a better way to register resources? Would prefer to do it once versus n+1 times. Also, is there is a reason it's n+1?

________________________________ In README.md<https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Falysivji%2Ffalcon-apispec%2Fpull%2F23%23discussion_r453245856&data=02%7C01%7Ceric.benson%40grainger.com%7Ce336f8a04ae542737b3408d825f32349%7C48d1dcb6bccc4365ac7fb937a7f7fd71%7C0%7C0%7C637301073506730807&sdata=GkLujQ4bzLPb31NulAkYEo%2BsnkuFjc8s45CnSFJAtcU%3D&reserved=0>:

+spec.path(resource=random_pet_resource)

spec.path(resource=random_pet_resource) resource=pet_resource since it got renamed — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Falysivji%2Ffalcon-apispec%2Fpull%2F23%23pullrequestreview-446835157&data=02%7C01%7Ceric.benson%40grainger.com%7Ce336f8a04ae542737b3408d825f32349%7C48d1dcb6bccc4365ac7fb937a7f7fd71%7C0%7C0%7C637301073506730807&sdata=CyynEYDMdoPG4ybih8tv6unQUjIs9JiYeaHXl5FG1ew%3D&reserved=0>, or unsubscribe<https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAJ5IWGB7VLHLBBKDF25ES73R3DZNFANCNFSM4OJTMJ7A&data=02%7C01%7Ceric.benson%40grainger.com%7Ce336f8a04ae542737b3408d825f32349%7C48d1dcb6bccc4365ac7fb937a7f7fd71%7C0%7C0%7C637301073506740803&sdata=uucfY8SfwoJibZN%2Flb3RNf1XTYmvn2mOw%2FgR6Vyrc2c%3D&reserved=0>.

[jeff-li]

Thanks for creating this PR! Is there any update regarding when this can be merged?

[alysivji]

Thanks for the ping. This slipped off my radar as other priorities came up.

I'm not a fan of the n+1 calls required for this feature to work properly. Adding a different abstraction on top or maybe doing a loop inside of the helper function would improve design.

I would need to take a closer look at what apispec is doing. I can take a look this weekend / early October (have some time devoted to Open Source during Hacktober) to see if I can come up with a better API.

[jeff-li]

thanks!