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

Memory exhaust 1024M in queued export with FromQuery method #4044

Conversation

sebestenyb
Copy link
Contributor

1️⃣ Why should it be added? What are the benefits of this change?

Please refer ticket #2209

In order to use LazyCollection in a FromCollection export, we need to expand the return type of the QueuedWriter class. Since we can't use union types due the support of older PHP version, we need to remove the return type, and do it via doc blocks again.

2️⃣ Does it contain multiple, unrelated changes? Please separate the PRs out.

No

3️⃣ Does it include tests, if possible?

No

4️⃣ Any drawbacks? Possible breaking changes?

No

5️⃣ Mark the following tasks as done:

  • Checked the codebase to ensure that your feature doesn't already exist.
  • Take note of the contributing guidelines.
  • Checked the pull requests to ensure that another person hasn't already submitted a fix.
  • Added tests to ensure against regression.

6️⃣ Thanks for contributing! 🙌

@sebestenyb sebestenyb changed the title Update QueuedWriter.php Memory exhaust 1024M in queued export with FromQuery method Dec 5, 2023
@patrickbrouwers
Copy link
Member

patrickbrouwers commented Dec 8, 2023

We should be able to add a test for this, no? If we would add a test right now with using a lazycollection the test would fail on a type error, right? After this PR that test should succeed. Would then prevent regressions of this breaking again in the future.

@sebestenyb
Copy link
Contributor Author

I've added a test, which fails if I restore the original QueuedWriter, and it runs correctly locally, but it fails in GH Actions for some reason.

@patrickbrouwers
Copy link
Member

patrickbrouwers commented Jan 9, 2024

I'll have a look at GH actions

@patrickbrouwers patrickbrouwers merged commit af25dd3 into SpartnerNL:3.1 Jan 9, 2024
1 of 16 checks passed
@patrickbrouwers
Copy link
Member

GH actions pass now

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.

None yet

2 participants