Issue 479: Edited api.content.get_view for increased efficiency to get views #528

samriddhi99 · 2024-01-31T05:45:54Z

The function does some avoidable check to wake up the view. I have used a try-except block avoid doing the check unless there is an error that pops up.
FIxes #479

mister-roboto · 2024-01-31T05:46:02Z

@samriddhi99 you need to sign the Plone Contributor Agreement to merge this pull request.

Learn about the Plone Contributor Agreement: https://plone.org/foundation/contributors-agreement

mister-roboto · 2024-01-31T05:46:05Z

@samriddhi99 thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

samriddhi99 · 2024-01-31T05:48:34Z

@jenkins-plone-org please run jobs

src/plone/api/content.py

mister-roboto · 2024-02-01T13:12:34Z

@samriddhi99 you need to sign the Plone Contributor Agreement to merge this pull request.

Learn about the Plone Contributor Agreement: https://plone.org/foundation/contributors-agreement

gforcada · 2024-02-04T08:47:58Z

@samriddhi99 thanks for this contribution, hopefully we get all the ticks green ✅ and can merge it soon! 🤩

There is one tick that you need to work on though: the contributor agreement, please read about it and sign it 🤞🏾 https://plone.org/foundation/contributors-agreement

gforcada · 2024-02-04T08:51:12Z

src/plone/api/content.py

+                    views="\n".join(sorted(available_view_names)),
+                ),
+            )
+    return adapter


adapter here could be not defined, if we get an exception when trying to get the adapter, and thus get into the exception Exception part of the code and the view name it is found in the available_view_names.

Could that happen, though? 🤔 A view that you don't have permissions for would get you in such corner case? 🤔

The security is not checked when you call getMultiAdapter, anyway the except Exception is not good IMO.

stevepiercy · 2024-02-04T10:29:30Z

@gforcada it appears @samriddhi99 was approved and added to the Contributors team after mister-roboto sent the reminder.

stevepiercy · 2024-02-04T10:36:19Z

Something is not quite right. @samriddhi99 was added to the Plone Contributors Team in GitHub, but mister-roboto says otherwise. It's possible that the emails in @samriddhi99's GitHub account do not align with what they put on their Agreement.

@samriddhi99 would you please add the email you put on your agreement to your GitHub account? If you already did that, it is possible that there is a typo somewhere, and you should contact [email protected], copying and pasting a link to this issue in reference to this issue.

ale-rt · 2024-02-05T07:57:12Z

src/plone/api/content.py

-            ),
+    try:
+        adapter = getMultiAdapter((context, request), name=name)
+    except Exception: 


I think this should be except ComponentLookupError, otherwise you risk to return a wrong message.

Thank you for pointing it out! I have made the changes accordingly.

ale-rt · 2024-02-05T07:57:14Z

src/plone/api/content.py

+                    views="\n".join(sorted(available_view_names)),
+                ),
+            )
+    return adapter


The security is not checked when you call getMultiAdapter, anyway the except Exception is not good IMO.

ale-rt

LGTM, I added a couple of suggestions to have micro-optimizations.
Thanks for your contribution!

src/plone/api/content.py

samriddhi99 · 2024-02-05T11:34:48Z

@stevepiercy
The plone contributors agreement verifier check has passed, so I think the issue has been resolved.

stevepiercy · 2024-02-05T11:41:32Z

@samriddhi99 thank you, and I confirm. You're not the only person that experienced a similar issue.

After all other checks pass, please read and follow this comment: #528 (comment)

samriddhi99 · 2024-02-05T11:42:18Z

@jenkins-plone-org please run jobs

src/plone/api/content.py

Co-authored-by: Alessandro Pisa <[email protected]>

davisagli

Thanks for this contribution!

Branch: refs/heads/master Date: 2024-01-29T23:26:21+05:30 Author: Samriddhi Singh (samriddhi99) <[email protected]> Commit: plone/plone.api@4b3ef60 Edited api.content.get_view for increased efficiency to get views Files changed: A news/479.internal M src/plone/api/content.py Repository: plone.api Branch: refs/heads/master Date: 2024-02-01T18:41:52+05:30 Author: Samriddhi Singh (samriddhi99) <[email protected]> Commit: plone/plone.api@f89a207 Changes as per plone/plone.api#528 (comment) have been made Files changed: M src/plone/api/content.py Repository: plone.api Branch: refs/heads/master Date: 2024-02-05T16:56:47+05:30 Author: Samriddhi Singh (samriddhi99) <[email protected]> Commit: plone/plone.api@d2715ad Error handling if component not found in function get_view Files changed: M src/plone/api/content.py Repository: plone.api Branch: refs/heads/master Date: 2024-02-05T17:10:06+05:30 Author: Samriddhi Singh (samriddhi99) <[email protected]> Commit: plone/plone.api@913b2cd Minor changes to get_view Files changed: M src/plone/api/content.py Repository: plone.api Branch: refs/heads/master Date: 2024-02-07T21:17:07-08:00 Author: David Glick (davisagli) <[email protected]> Commit: plone/plone.api@2e80155 Update src/plone/api/content.py Co-authored-by: Alessandro Pisa <[email protected]> Files changed: M src/plone/api/content.py Repository: plone.api Branch: refs/heads/master Date: 2024-02-07T21:21:08-08:00 Author: David Glick (davisagli) <[email protected]> Commit: plone/plone.api@3565a0b Merge pull request #528 from samriddhi99/Issue-479-optimizing-api.content.get_view Issue 479: Edited api.content.get_view for increased efficiency to get views Files changed: A news/479.internal M src/plone/api/content.py

Edited api.content.get_view for increased efficiency to get views

4b3ef60

davisagli requested changes Jan 31, 2024

View reviewed changes

src/plone/api/content.py Outdated Show resolved Hide resolved

src/plone/api/content.py Outdated Show resolved Hide resolved

Changes as per plone#528 (comment) have been made

f89a207

gforcada reviewed Feb 4, 2024

View reviewed changes

stevepiercy requested review from davisagli and gforcada February 4, 2024 10:29

ale-rt reviewed Feb 5, 2024

View reviewed changes

Error handling if component not found in function get_view

d2715ad

ale-rt reviewed Feb 5, 2024

View reviewed changes

src/plone/api/content.py Outdated Show resolved Hide resolved

src/plone/api/content.py Outdated Show resolved Hide resolved

Minor changes to get_view

913b2cd

ale-rt reviewed Feb 5, 2024

View reviewed changes

src/plone/api/content.py Outdated Show resolved Hide resolved

ale-rt approved these changes Feb 5, 2024

View reviewed changes

Update src/plone/api/content.py

2e80155

Co-authored-by: Alessandro Pisa <[email protected]>

davisagli approved these changes Feb 8, 2024

View reviewed changes

davisagli merged commit 3565a0b into plone:master Feb 8, 2024
6 of 10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 479: Edited api.content.get_view for increased efficiency to get views #528

Issue 479: Edited api.content.get_view for increased efficiency to get views #528

samriddhi99 commented Jan 31, 2024

mister-roboto commented Jan 31, 2024

mister-roboto commented Jan 31, 2024

samriddhi99 commented Jan 31, 2024

mister-roboto commented Feb 1, 2024

gforcada commented Feb 4, 2024

gforcada Feb 4, 2024

ale-rt Feb 5, 2024

stevepiercy commented Feb 4, 2024

stevepiercy commented Feb 4, 2024 •

edited

Loading

ale-rt Feb 5, 2024

samriddhi99 Feb 5, 2024

ale-rt Feb 5, 2024

ale-rt left a comment

samriddhi99 commented Feb 5, 2024

stevepiercy commented Feb 5, 2024

samriddhi99 commented Feb 5, 2024

davisagli left a comment

Issue 479: Edited api.content.get_view for increased efficiency to get views #528

Issue 479: Edited api.content.get_view for increased efficiency to get views #528

Conversation

samriddhi99 commented Jan 31, 2024

mister-roboto commented Jan 31, 2024

mister-roboto commented Jan 31, 2024

samriddhi99 commented Jan 31, 2024

mister-roboto commented Feb 1, 2024

gforcada commented Feb 4, 2024

gforcada Feb 4, 2024

Choose a reason for hiding this comment

ale-rt Feb 5, 2024

Choose a reason for hiding this comment

stevepiercy commented Feb 4, 2024

stevepiercy commented Feb 4, 2024 • edited Loading

ale-rt Feb 5, 2024

Choose a reason for hiding this comment

samriddhi99 Feb 5, 2024

Choose a reason for hiding this comment

ale-rt Feb 5, 2024

Choose a reason for hiding this comment

ale-rt left a comment

Choose a reason for hiding this comment

samriddhi99 commented Feb 5, 2024

stevepiercy commented Feb 5, 2024

samriddhi99 commented Feb 5, 2024

davisagli left a comment

Choose a reason for hiding this comment

stevepiercy commented Feb 4, 2024 •

edited

Loading