arangodb-php

arangodb-php copied to clipboard

I have rewritten FoxxHandler to use the foxx-api as it is documented.

Added abilities for install:

• pass 'configuration' and 'dependencies'
• set 'legacy', 'setup', 'teardown', 'development' options

Added abilities for removal:

• set 'teardown' option

I have added a multipart/form-data body assembling method to HttpHelper, and changed HttpHelper::buildRequest() to honor the 'Content-Type' header if passed in $customHeaders.

I extracted the basic option handling code from ConnectionOptions to reuse it for the Multipart handling.

If this is basically acceptable, I would add more of the api Methods (update, dependency and configuration access). I started adding them nonetheless, it's quickly done.

I deprecated the old method names and opted for a more streamlined, api-matching method naming scheme (e.g. removeFoxxApp -> uninstallService).

A signed contributor agreement should exist.

My calling code works as is; I have php-8 related problems with the code-base (strict type handling) - I opted for correcting the singatures, if that's bad, I'll happily add the #[Returntypechanges] attribute where needed.

I'll check the CI-log and fix what I can tomorrow

The remaining 6 failures are not caused by my changes.

It seems to me that they are caused by changes on the server side.

~~The documentation states, that PUT and PATCH are to be used to replace resp. update a foxx service; implemented that way, the server (3.9.3) responds with 405 Method not allowed. I the documentation wrong, or the server?~~ I found my mistake (always used install url).

A good portion of the last failures were caused by my changes (timeout handling); I adapted the other failing tests to accept the behaviour of the server.

Sorry - this PR is a bit messier than anticipated.

Three weeks with no reaction - as I took the time to make sure all tests pass, adding new tests, etc. I would appreciate a bit of your time - a get outta here in time is better than hanging in limbo, for know I'm waiting on your verdict - if I know there'll be none, I can consider my options wrt/ our projects, that would like to (in part at least) rely on this PR.

@tomterl : sorry, have overlooked the action here. A colleague just pointed me at this. Will have a look today/tomorrow and get back. Thanks already!

Thanks! Take your time; I just wanted to make sure it doesn't fall through the cracks.

@tomterl : thanks a lot for this PR. I tried running the Foxx tests in your branch, and the tests worked fine using ArangoDB 3.8, 3.9, 3.10 and 3.11/devel as backends.

I saw that the PR contains changes to some unrelated test files:

commit a07135f76a8613ebb4ef45b5f9dd4be0f31f0401
Author: Tom Regner <[email protected]>
Date:   Wed Oct 5 10:16:46 2022 +0200

    Change StatementTest to accept the servers behaviour.

 tests/StatementTest.php | 15 ---------------
 1 file changed, 15 deletions(-)

commit 96c2ac9f6a652d5996a70dfc66e00cf684b3206d
Author: Tom Regner <[email protected]>
Date:   Wed Oct 5 10:13:34 2022 +0200

    Change AdminTest to accept the servers behaviour.

 tests/AdminTest.php | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

I think these changes should be reverted, because they would actually fail when running the tests again later versions of arangod.

The issue seems to be caused by the arangod backend version number in https://github.com/arangodb/arangodb-php/blob/devel/tests/travis/setup_arangodb.sh#L23-L24 not having been updated for a while, but I think increasing the version number there should fix the (unrelated) tests already. I am happy if this is done in a separate PR and I can do this. But I would still prefer to revert the changes to the unrelated PRs from this one.

Apart from that, this PR targets devel, which is the current ArangoDB version 3.11. If you want this fix to appear in an earlier version (say 3.9 or 3.10) it will need to be backported. I can do this easily once your PR is merged into devel.

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Tom Regner. This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email [email protected]
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

Happy to oblige, I reverted to two commits that changed the tests - I assumed the test-server was more recent then the tests, not the other way around.

The backporting: That would be nice (3.8, 3.9, 3.10), if it is easier/less work for you when I prepare PRs for the branches, tyhen I'll do that, if it's all the same to you, I wouldn't mind if you did it without my intervention.

@tomterl: The test failures that happened in the last build are "expected" and unrelated to the changes in this PR. I will create a separate PR to address them properly. I will now merge this PR into devel. Do you need backports for any previous versions?

The backporting: That would be nice for (3.8, 3.9, 3.10), if it is easier/less work for you when I prepare PRs for the branches, tyhen I'll do that, if it's all the same to you, I wouldn't mind if you did it without my intervention.

@tomterl : I have updated the arangod images used by the TravisCI tests, so that now the tests should pass in branches 3.8, 3.9, 3.10 and devel without having to adjust them in PRs. If you want to backport the FoxxHandler changes to any of these branches, please open PRs as you see fit. Thanks a lot!