Skip to content

Conversation

@dakotawashok
Copy link
Contributor

Enhancement: New PHP 8.2 Hello World Examples with Nginx, Docker-Compose, and DevCycle Proxy

This Pull Request introduces two new Hello World applications for our PHP SDK, setup with PHP 8.2 and integrated with nginx, docker-compose, and the DevCycle Local Bucketing Proxy.

Key Changes:

  1. New Hello World Applications: Two examples added to demonstrate the PHP SDK's functionality with the DevCycle Local Bucketing Proxy in a PHP 8.2 environment.
  2. PHP 8.2 Compatibility: These containers assume PHP 8.2 is being used
  3. Nginx Integration: Nginx is used as the web server
  4. Docker-Compose Setup: The environment setup is streamlined with docker-compose for ease of use and consistency.
  5. DevCycle Local Bucketing Proxy Integration:

Looking forward to your feedback on these enhancements.

Dakota Washok
Senior Software Engineer/DevOps Engineer

@dakotawashok
Copy link
Contributor Author

Looks like there are some failing tests reported by the Harness Tests suite? I'm not sure if those are okay or not, but let me know if you want me to update anything!

@jonathannorris
Copy link
Member

Hey @dakotawashok thanks so much for submitting this PR! Our "virtual office" is closed this week, we will be sure to dig into it next week when everyone is back.

@JamieSinn
Copy link
Member

Further investigation shows this is due to a file permissions issue for the raw socket file:
DevCycleHQ/sdk-proxy#12 (comment)

This is an awesome contribution for the repo!

@dakotawashok
Copy link
Contributor Author

Hello @JamieSinn / @jonathannorris !
I'm not sure who to ask this, so I figured I'd ping both, sorry about that.
We decided to use the HTTP SDK proxy instead of the UDP proxy for the time being, until we can further investigate our setup and how to integrate the proper file/group permissions across our services.
The HTTP SDK Proxy seems to be working like a charm, though! Thanks for that :)

As for this PR, though, I still think it'd be good to have a variety of example setups in this project for others to use and build off of. If yall agree, I'd love to get this in here still. Is there anything you need me to update on my end?

@JamieSinn
Copy link
Member

Absolutely, I'd love to get it in here.

I think for tweaks, making the socket available under 777 would be a quick fix, somehow part of the docker compose setup or a docker file edit.

With that, it works and we can merge the rest as is!

fix: alter devcycle udp socket file permissions in example app
@dakotawashok
Copy link
Contributor Author

Hey @JamieSinn I've altered the Supervisord configuration in the UDP example app to alter the socket file permissions after startup. Please take a look and let me know if you want anything else updated!

@JamieSinn
Copy link
Member

Hey @dakotawashok - are you able to update the example apps to use the new init flow?
Check the readme if you need help

@dakotawashok
Copy link
Contributor Author

@JamieSinn I'll take a look and integrate it with these example apps once I'm able to hop back onto this, sorry for the wait!

@JamieSinn
Copy link
Member

No rush! Thank you!

@JamieSinn JamieSinn self-assigned this Feb 6, 2024
@dakotawashok
Copy link
Contributor Author

I've got the issues you've pointed out resolved, could you take another look at this @JamieSinn ? I'll test again in the morning, but I think these changes are good to go! Let me know if you'd like me to resolve the unit tests that are failing, as I can't remember if they were a part of the Checks in this repo when I originally opened this haha

@JamieSinn
Copy link
Member

Looks great, I'll take a shot at testing it when I'm in the office

@JamieSinn
Copy link
Member

Seems like supervisord is crashing still

image Did it work when you tried it locally? I'm using a test token so it may be a weird environment setup

@dakotawashok
Copy link
Contributor Author

dakotawashok commented Feb 13, 2024

Looks good now! Here's the result from variable.php for a variable set to true in the http proxy example:
image

Here's the result from variable.php for a variable set to true in the udp proxy example:
image

@dakotawashok
Copy link
Contributor Author

@JamieSinn can you please try this again, replacing the sdk in the docker-compose files with a proper key? This should be ready for review.

{
"sdkKey": "${DEVCYCLE_SDK_KEY}",
"unixSocketPath": "/tmp/dvc_lb_proxy.sock",
"unixSocketEnabled": true,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"unixSocketEnabled": true,
"unixSocketEnabled": true,
"unixSocketPermissions": 777,

You'll need to set the file perms here too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the extra commands from the supervisord configuration and I'm testing this functionality now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JamieSinn Is this configuration option supposed to negate the need to alter the socket file permissions after the proxy command is run?

Copy link
Member

Choose a reason for hiding this comment

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

yep!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not able to get the actual variable value when using this configuration option and removing the extra commands for altering the socket file permissions, which is why I ask.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm - that's odd - let me do a quick test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll push up the changes I used to test that, I think that'd help, and I can resolve any issues afterward.

Copy link
Member

Choose a reason for hiding this comment

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

sounds good. I have a feeling this might be on my implementation.

Copy link
Member

Choose a reason for hiding this comment

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

Yep - looks like that was on me. I fixed it to now support octal perms. 1.1.1 will fix it

@dakotawashok
Copy link
Contributor Author

@JamieSinn I'm still getting an issue, even with updating the proxy package to 1.1.1. Here's the error I get when running the command now, a new error:

2024/02/13 21:08:17 Loading configuration from file: /etc/devcycle-sdk-proxy/dvc-proxy-config.json
2024/02/13 21:08:17 Failed to parse config: failed to parse config from JSON: json: cannot unmarshal number into Go struct field ProxyInstance.instances.unixSocketPermissions of type string
2024/02/13 21:08:17 Please either set the config path or set the environment variables

I tried "unixSocketPermissions": "777", and "unixSocketPermissions": 777, in my dvc-proxy-config.json.

Here's the results of the udp variable test. Interestingly, the webpage still is loading without a devcycle error being reported, even though the proxy isn't apparently running:
image

@JamieSinn
Copy link
Member

JamieSinn commented Feb 13, 2024

@JamieSinn I'm still getting an issue, even with updating the proxy package to 1.1.1. Here's the error I get when running the command now, a new error:

2024/02/13 21:08:17 Loading configuration from file: /etc/devcycle-sdk-proxy/dvc-proxy-config.json
2024/02/13 21:08:17 Failed to parse config: failed to parse config from JSON: json: cannot unmarshal number into Go struct field ProxyInstance.instances.unixSocketPermissions of type string
2024/02/13 21:08:17 Please either set the config path or set the environment variables

I tried "unixSocketPermissions": "777", and "unixSocketPermissions": 777, in my dvc-proxy-config.json.

Here's the results of the udp variable test. Interestingly, the webpage still is loading without a devcycle error being reported, even though the proxy isn't apparently running: image

ah my bad - I forgot to say you'll need to change that param to be a string, "0777" specifically.
The loading without an error is the expected behaviour - devcycle should default and not bubble the error to the user layer.

@dakotawashok
Copy link
Contributor Author

Woohoo! That worked! I'm able to get our variable successfully now on both proxy hello world examples. If this is good to go for yall, please feel free to merge in.

@JamieSinn JamieSinn merged commit 8a40424 into DevCycleHQ:main Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants