Skip to content
This repository was archived by the owner on Oct 19, 2023. It is now read-only.

Stackdriver integration defer check#431

Merged
chingor13 merged 8 commits intomasterfrom
stackdriver-integration-defer-check
Apr 23, 2018
Merged

Stackdriver integration defer check#431
chingor13 merged 8 commits intomasterfrom
stackdriver-integration-defer-check

Conversation

@chingor13
Copy link
Member

@chingor13 chingor13 commented Apr 19, 2018

Defer checking the google/cloud library versions until after composer has resolved and installed dependencies. Rather that just looking for the prepend file after composer installation, we also verify google/cloud versions.

Also, fixes and runs the php-base tests as part of CI.

Fixes #426

@chingor13 chingor13 changed the title [WIP] Stackdriver integration defer check Stackdriver integration defer check Apr 19, 2018
@chingor13 chingor13 requested a review from tmatsuo April 19, 2018 16:07
$location = $integration->prependFileLocation();
$fp = fopen($options['o'], 'w');
try {
fwrite($fp, 'auto_prepend_file=' . $location . PHP_EOL);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think fwrite fails without throwing an exception. Maybe check the return value and throw?

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense - will add a check.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed - checking both fopen and fwrite

'o' => 'php://stdout'
];

$autoload = $options['a'] . '/vendor/autoload.php';
Copy link
Contributor

Choose a reason for hiding this comment

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

It will fail when you're using non-standard vendor-dir. Can we ignore this case?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we can ignore this for now. The way it currently works will also fail with a non-standard vendor-dir because we have hard-coded fallback paths under /app/vendor/....

It looks like we can query for the vendor directory with composer config vendor-dir, but we can do this in a separate PR.

@chingor13 chingor13 merged commit fdbfdc0 into master Apr 23, 2018
@chingor13 chingor13 deleted the stackdriver-integration-defer-check branch April 23, 2018 22:41
frost-byte pushed a commit to Wiser-Owl/php-docker that referenced this pull request May 9, 2021
* Defer checking stackdriver integration required versions until composer has resolved dependencies

* Fix code style

* Remove gen dockerfile tests

* Add base tests for stackdriver integration

* Run the php-base tests. Add tests for version checking google/cloud libraries

* Fix phpcs

* Move no_composer test to php-base

* Throw and log error if we cannot open or write the php.ini config
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stackdriver Integration version detection too strict

3 participants