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

Implements IDateRangeIndex to exclude DateRecurringIndex by indexes with value in the keys of the catalog plan #8

Merged
merged 6 commits into from
Jan 13, 2025

Conversation

mamico
Copy link
Contributor

@mamico mamico commented Jul 21, 2022

During analysis on ZCatalog plan (zopefoundation/Products.ZCatalog#138), I saw many entries in the catalog plan with key as

(('start', "{'query': datetime.datetime(2010, 10, 10, 0, 0), 'range': 'min'}"),)

(where in my use case start is a DateRecurringIndex)

The fix proposal here avoids this behavior (change here: https://github.com/collective/Products.DateRecurringIndex/compare/mamico/catalogplan?expand=1#diff-2e7e66d717198090a3b8cd8c0152b953b7f12d782a0a46cc381776e6608be5ceR22)

In this PR there is also GitHub action to automate testing.

@mister-roboto

This comment was marked as resolved.

@mamico mamico requested review from jensens and mauritsvanrees July 21, 2022 11:40
@mamico
Copy link
Contributor Author

mamico commented Jul 21, 2022

@jenkins-plone-org please run jobs

@mamico mamico changed the title Implements IDateRangeIndex so is excluded by index that could have value in the keys of the catalog plan Implements IDateRangeIndex to exclude DateRecurringIndex by indexes that could have value in the keys of the catalog plan Jul 22, 2022
@mamico mamico changed the title Implements IDateRangeIndex to exclude DateRecurringIndex by indexes that could have value in the keys of the catalog plan Implements IDateRangeIndex to exclude DateRecurringIndex by indexes with value in the keys of the catalog plan Jul 22, 2022
import unittest
from datetime import datetime

import pytz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

importing here will slow down whenever this module is imported - which is on test discovery, IIRC.
normally, pytz needs some time to be imported.
however, i'll remove this dependency in an upcoming PR, so it's fine to merge here!

@mamico mamico merged commit 5e6bc96 into master Jan 13, 2025
2 of 4 checks passed
@mamico mamico deleted the mamico/catalogplan branch January 13, 2025 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants