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

Question about OffsetFilter / step priorities #249

Closed
gnat42 opened this issue Aug 6, 2015 · 1 comment
Closed

Question about OffsetFilter / step priorities #249

gnat42 opened this issue Aug 6, 2015 · 1 comment

Comments

@gnat42
Copy link
Contributor

gnat42 commented Aug 6, 2015

So I have some code that works and has a number of unit tests. However it consumes the entire source file. We're converting it to be processed via batches. To do so I figured I'd simply add the OffsetFilter before all other steps and it would be easy. This fails somehow.

So for example my processor has the following function

        $workflow->addStep($import->getMappings());
        $workflow->addStep($import->getIgnoredMapper());

        $valueConverter = new ValueConverterStep();
        $valueConverterCount = 0;
        foreach ($import->getConverters() as $column) {
            $name = ($column->hasMapper()) ? $column->getMapper() : $column->getName();
            $valueConverter->add(sprintf('[%s]', str_replace('.', '][', $name)), $this->container->get($column->getConverter()));
            $valueConverterCount++;
        }

        if ($valueConverterCount > 0) {
            $workflow->addStep($valueConverter);
        }

        $filterStep = new FilterStep();
        $addFilter = false;

        if ($this->notBlankFilter) {
            $filterStep->add($this->notBlankFilter);
            $addFilter = true;
        }

        if ($this->duplicateFilter) {
            $filterStep->add($this->getDuplicate());
            $addFilter = true;
        }

        if ($addFilter) {
            $workflow->addStep($filterStep);
        }

This works.

Now I was trying to add the offset filter. Suddenly tests fail in weird ways. Thinking perhaps I wasn't doing something properly. I added the following which from what I read should really do nothing, everything still fails.

        $offsetFilter = new FilterStep();
        $offsetFilter->add(new OffsetFilter());
        $workflow->addStep($offsetFilter);
...
        $workflow->addStep($import->getMappings());
        $workflow->addStep($import->getIgnoredMapper());

What's going on? I started digging into it a bit and thought perhaps it was related to the priority handling code. I couldn't find any documentation on the SplPriorityQueue and what the priority value means. For example is 1 run before 2? Looking at some of the tests, one test had the offset filter added with priority 257, but that was the FilterStep priority, not the StepAggregator priority.

In any case I would expect adding an empty offset filter would not affect the output at all.

@gnat42
Copy link
Contributor Author

gnat42 commented Aug 6, 2015

So I did a bit more digging and realized once I applied the fix in PR #248 and called addStep passing a priority. The higher the number the higher the priority. So now my tests pass however I find it quite bizarre that the way this is implemented is somewhat reversed. I would think if I don't specify any priority the steps should be run in the order they are added. That should be made really clear in the docs when they are updated to match the new step interface.

@gnat42 gnat42 closed this as completed Aug 6, 2015
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

No branches or pull requests

1 participant