8000 Added ability to limit number of extracted records by rtripault · Pull Request #418 · modmore/Gitify · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Added ability to limit number of extracted records #418

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

Merged
merged 2 commits into from
Mar 23, 2023

Conversation

rtripault
Copy link

What does it do ?

Introduced a new limit option (only used on extract) to... limit the number of extracted records

Why is it needed ?

We are having large projects were all the records are not needed (ie. all blog entries) but only a subset for testing/development purpose. With that limit option, we can safely extract only x records.

Related issue(s)/PR(s)

None found

@@ -201,6 +201,9 @@ public function extractObjects($folder, array $options = array())
if (!empty($criteria)) {
$c->where(array($criteria));
}
if (isset($options['limit']) && is_int($options['limit'])) {
$c->limit($options['limit']);
}
Copy link
Author

Choose a reason for hiding this comment

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

If that's something you are happy to consider, maybe we should also allow some sorting options (ie. to only process higher IDs/most recent entries)

@Mark-H
Copy link
Member
Mark-H commented Dec 23, 2022

Won't this cause excluded objects to get removed from the data directory entirely?

Though I guess you're thinking of using this with #417 to have separate data directories, e.g. one for dev and one for prod?

@rtripault
Copy link
Author

Won't this cause excluded objects to get removed from the data directory entirely?

Though I guess you're thinking of using this with #417 to have separate data directories, e.g. one for dev and one for prod?

You are correct on both statement. But there indeed might be drawbacks we did not consider for our use case

@rtripault
Copy link
Author

Quick note to let you know this needs more work (at least to fulfill our own requirements) for the "content" type extraction

@muzzwood
Copy link
Contributor

Quick note to let you know this needs more work (at least to fulfill our own requirements) for the "content" type extraction

Hi @rtripault
Just wanted to check if you were happy with this PR now (after your commit on Jan 6), or if you were planning any more changes?

@rtripault
Copy link
Author

Hi @rtripault Just wanted to check if you were happy with this PR now (after your commit on Jan 6), or if you were planning any more changes?

Hi there! Sorry for not commenting! We are happy with it in its current state (hopefully you folks will be too!)

@muzzwood
Copy link
Contributor

Thanks a lot, @rtripault !
I'll test it out properly tomorrow :)

@muzzwood
Copy link
Contributor

Works great! Nice work as always @rtripault !
Though, there's a little discrepancy with the limitPerParent() phpdocs saying the options to use are sort_by and sort_dir, when the method itself uses order_by and order_dir. I'll merge this and then I might change the naming to "sort_" so it matches the rest.
Thanks for contributing! 🚀

@muzzwood muzzwood merged commit 21886b1 into modmore:master Mar 23, 2023
@wuuti wuuti deleted the extract-limit branch July 11, 2023 12:34
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.

3 participants
0