Fix the memory limit check #404
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Well, I don't think it even make sense to check memory_limit there. It looks like some old temporary fix. I guess it's better to show body only for jsons and truncate it by like 5kb. |
I don't think it makes sense either, but when it comes to someone else's codebase (and one that I am unfamiliar with) I tend to lean towards fixing a bug rather than modifying existing behavior. I honestly am unable to think of any valid situation for this check to exist at all truthfully Edit: tried to find when it was added via git blame to see if there was an explanation behind it, but it predates this code being on GitHub so I haven idea |
Merged
k0ka
added a commit
that referenced
this pull request
Feb 10, 2024
giogurto-grande
added a commit
to VS-CDeX/openstack
that referenced
this pull request
Jun 19, 2024
* GitHub workflows (php-opencloud#369) Add integration workflows for services: - BlockStorage - Compute - Images - Networking * always recheck all tests on push to master (php-opencloud#370) * Restore read the docs (php-opencloud#371) * add readthedocs.yaml * remove button * move to python 3 * Fix documentation (php-opencloud#372) * fix php-opencloud#351 * fix documentation requirements, remove unused links in docs * add badges, remove old contacts from README.md * Fix documentation (php-opencloud#373) * clarify versioning * change documentation copyright * Fix sort_key and sort_dir parameters for BlockStorage/v2/listSnapshots and Images/v2/listImages * fix list formating in documentation (php-opencloud#375) remove custom theme * added phpdocs for better type hinting (php-opencloud#376) * one integration workflow (php-opencloud#377) * one integration workflow to rule them all * update README.md with new badge * create BlockStorage v3 as copy of v2 (php-opencloud#378) * Chores (php-opencloud#379) * format via php-cs-fixer * add unit tests with the lowest possible dependencies * add unit tests with php 8.2 and 8.3 * add integration tests for 2023.1 antelope and yoga * allow skipping integration, unit or both tests in pull requests --------- Co-authored-by: k0ka <k0ka@users.noreply.github.com> * Application credentials (php-opencloud#380) * add endpoints to create/get/remove application credentials * add token creation using application credentials * cancel running workflows on new commit --------- Co-authored-by: smarcet <smarcet@gmail.com> Co-authored-by: k0ka <k0ka@users.noreply.github.com> * fix doc for application credentials (php-opencloud#381) * Handler stack factory (php-opencloud#382) * Changed guzzle final HandlerStack extension to factory class HandlerStackFactory --------- Co-authored-by: k0ka <k0ka@users.noreply.github.com> Co-authored-by: jarragon-slash2 <jarragon-slash2@users.noreply.github.com> * Fix testing class namespaces (php-opencloud#383) Co-authored-by: peter279k <peter279k@gmail.com> * Refactor tests (php-opencloud#384) use `include_once $this->sampleFile(` instead of `$path = $this->sampleFile(..); include_once $path` * Move integration tests to phpunit (php-opencloud#387) * moved integration tests to phpunit * updated integration tests description in CONTRIBUTING.md * added `name` parameter to `patchUser` so the sample file works properly * added `Retrievable` interface to `VolumeType` * added `HasWaiterTrait` to `Compute::Image` * added `Token::validate()` function to check if Identity token is valid --------- Co-authored-by: k0ka <k0ka@users.noreply.github.com> * License: add full text, remove rackspace (php-opencloud#388) * added bigger rescue timeouts (php-opencloud#390) * return HandlerStack for BC (php-opencloud#391) * return HandlerStack for Backward Compatibility - php-opencloud#382 * Merge network services (php-opencloud#392) * Merge all network service extensions into main one using traits * Add unit test error_reporting * Increase volume attachment test timeout * Clarify docs (php-opencloud#389) * Rewrite most of documentation. * Make creating the `$openstack` object more clear. * Rename and rearrange main operations as CRUDL (Create, Read, Update, Delete, List) * Resume suspend server (php-opencloud#394) * Implement resuming and suspending of servers --------- Co-authored-by: Martin Zurowietz <martin@zurowietz.de> * fix resume/suspend doc (php-opencloud#395) * refactor unit tests: use `mockRequest` for all requests (php-opencloud#397) * Fix Swift container requests with "tokens" in its name (php-opencloud#396) * add errorVerbosity (php-opencloud#400) fixes php-opencloud#335 fixes php-opencloud#398 * add docs for volume attachement (php-opencloud#401) fixes php-opencloud#399 * fix doc links (php-opencloud#402) * enchance error builder: output body only for json, limit body to 5 Kb (php-opencloud#405) fixes php-opencloud#403 supersedes php-opencloud#404 * make swift metadata header case insensitive (php-opencloud#407) * compare headers case insensitively * drop support of unmaintained releases in CI (until they are returned in github action). * return unmaintaned openstack versions into ci (php-opencloud#408) * override TARGET_BRANCH * Apply php-cs-fixer changes --------- Co-authored-by: Konstantin Babushkin <koka@idwrx.com> Co-authored-by: k0ka <k0ka@users.noreply.github.com> Co-authored-by: smarcet <smarcet@gmail.com> Co-authored-by: jarragon-slash2 <jarragon-slash2@users.noreply.github.com> Co-authored-by: peter279k <peter279k@gmail.com> Co-authored-by: Martin Zurowietz <martin@zurowietz.de> Co-authored-by: Kamil Kozłowski <kamil.kozlowski@cdex.cloud> Co-authored-by: giogurto-grande <giogurto-grande@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #403
When you query
ini_get('memory_limit')you don't always get an integer response, it returns exactly what is written inside of the PHP.inifile file which typically includes a shorthand value. (K,M, orG, all case-insensitive).My server has the default PHP limit of
128Mwhich was incorrectly being evaluated as not enough by the check.This PR corrects the behavior by first evaluating the set memory limit, including the shorthand byte values and then returns it as an integer which can then be properly compared against the message's body size.