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

Added RR_VERSION env to Environment class #37

Merged
merged 6 commits into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .github/ISSUE_TEMPLATE/1_Bug_report.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ labels: Bug

### Additional Info

| Q | A
|------------------| ---
| Package Version | x.y.z <!-- Please set the package version -->
| PHP version | x.y.z <!-- Please set the PHP version -->
| Operating system | Linux <!-- Please set your OS -->
| Q | A |
|------------------|-----------------------------------------------|
| Package Version | x.y.z <!-- Please set the package version --> |
| PHP version | x.y.z <!-- Please set the PHP version --> |
| Operating system | Linux <!-- Please set your OS --> |

<!-- Optional: any other context about the problem: log messages, screenshots, etc. -->
14 changes: 7 additions & 7 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
| Q | A
| ------------- | ---
| Bugfix? | ✔️/❌
| Breaks BC? | ✔️/❌ <!-- please update "xxx Impact Changes" section in CHANGELOG.md file -->
| New feature? | ✔️/❌ <!-- please update "Other Features" section in CHANGELOG.md file -->
| Issues | #... <!-- prefix each issue number with "#" symbol, no need to create an issue if none exist, explain below instead -->
| Docs PR | spiral/docs#... <!-- prefix each issue number with "spiral/docs#", required only for new features -->
| Q | A |
|--------------|-------------------------------------------------------------------------------------------------------------------------|
| Bugfix? | ✔️/❌ |
| Breaks BC? | ✔️/❌ <!-- please update "xxx Impact Changes" section in CHANGELOG.md file --> |
| New feature? | ✔️/❌ <!-- please update "Other Features" section in CHANGELOG.md file --> |
| Issues | #... <!-- prefix each issue number with "#" symbol, no need to create an issue if none exist, explain below instead --> |
| Docs PR | spiral/docs#... <!-- prefix each issue number with "spiral/docs#", required only for new features --> |

Comment on lines +1 to +7
Copy link

Choose a reason for hiding this comment

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

Consider adding the missing articles to enhance grammatical correctness.

- | New feature? | ✔️/❌ <!-- please update "Other Features" section in CHANGELOG.md file -->
+ | New feature? | ✔️/❌ <!-- please update the "Other Features" section in CHANGELOG.md file -->

- | Issues       | #... <!-- prefix each issue number with "#" symbol, no need to create an issue if none exist, explain below instead -->
+ | Issues       | #... <!-- prefix each issue number with "#" symbol, no need to create an issue if none exist, explain below instead -->
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
| Q | A |
|--------------|-------------------------------------------------------------------------------------------------------------------------|
| Bugfix? | ✔️/❌ |
| Breaks BC? | ✔️/❌ <!-- please update "xxx Impact Changes" section in CHANGELOG.md file --> |
| New feature? | ✔️/❌ <!-- please update "Other Features" section in CHANGELOG.md file --> |
| Issues | #... <!-- prefix each issue number with "#" symbol, no need to create an issue if none exist, explain below instead --> |
| Docs PR | spiral/docs#... <!-- prefix each issue number with "spiral/docs#", required only for new features --> |
| Q | A |
|--------------|-------------------------------------------------------------------------------------------------------------------------|
| Bugfix? | ✔️/❌ |
| Breaks BC? | ✔️/❌ <!-- please update "xxx Impact Changes" section in CHANGELOG.md file --> |
| New feature? | ✔️/❌ <!-- please update the "Other Features" section in CHANGELOG.md file --> |
| Issues | #... <!-- prefix each issue number with "#" symbol, no need to create an issue if none exist, explain below instead --> |
| Docs PR | spiral/docs#... <!-- prefix each issue number with "spiral/docs#", required only for new features --> |
Tools
LanguageTool

[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ... | | New feature? | ✔️/❌ <!-- please update "Other Features" section in CHANGELOG.md ...


[uncategorized] ~6-~6: You might be missing the article “the” here.
Context: ... | | Issues | #... <!-- prefix each issue number with "#" symbol, no need to create an issue if...

<!-- Please, replace this notice by a short description of your feature/bugfix.
This will help people understand your PR and can be used as a start for the documentation. -->
14 changes: 10 additions & 4 deletions src/Environment.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@
/**
* @psalm-import-type ModeType from Mode
* @psalm-type EnvironmentVariables = array{
* RR_MODE?: ModeType|string,
* RR_RELAY?: string,
* RR_RPC?: string,
* RR_MODE?: ModeType|string,
* RR_RELAY?: string,
* RR_RPC?: string,
* RR_VERSION?: string,
* }|array<string, string>
* @see Mode
*/
Expand All @@ -27,7 +28,7 @@ public function __construct(

public function getMode(): string
{
return $this->get('RR_MODE', '');
return $this->get('RR_MODE');
}

public function getRelayAddress(): string
Expand All @@ -40,6 +41,11 @@ public function getRPCAddress(): string
return $this->get('RR_RPC', 'tcp://127.0.0.1:6001');
}

public function getVersion(): string
{
return $this->get('RR_VERSION');
}

/**
* @template TDefault of string
*
Expand Down
5 changes: 5 additions & 0 deletions src/EnvironmentInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,9 @@ public function getRelayAddress(): string;
* RPC address.
*/
public function getRPCAddress(): string;

/**
* Roadrunner version
*/
public function getVersion(): string;
Copy link
Member

Choose a reason for hiding this comment

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

@Kaspiman Adding a new method to the interface can break the user implementation of this interface. I suggest not adding the method to the interface, but only to the implementation, and indicating it in the interface as a comment:

/**
 * Provides base values to configure roadrunner worker.
 *
 * @psalm-import-type ModeType from Mode
 * @see Mode
 * @method string getVersion()
 */

The method can then be added in the next major release of the package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}
3 changes: 1 addition & 2 deletions src/Worker.php
Original file line number Diff line number Diff line change
Expand Up @@ -276,11 +276,10 @@ public static function createFromEnvironment(
);
}

private function sendProcessId(): static
private function sendProcessId(): void
{
$frame = new Frame($this->encode(['pid' => \getmypid()]), [], Frame::CONTROL);
$this->sendFrame($frame);
return $this;
}

private function haveToPing(): void
Expand Down
8 changes: 8 additions & 0 deletions tests/Unit/EnvironmentTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,23 @@ public function testGetRPCAddressWithValue(): void
$this->assertEquals('rpc_value', $env->getRPCAddress());
}

public function testGetVersionWithValue(): void
{
$env = new Environment(['RR_VERSION' => '2024.1.3']);
$this->assertEquals('2024.1.3', $env->getVersion());
}

public function testFromGlobals(): void
{
$_ENV['RR_MODE'] = 'global_mode';
$_SERVER['RR_RELAY'] = 'global_relay';
$_SERVER['RR_VERSION'] = 'global_version';

$env = Environment::fromGlobals();

$this->assertEquals('global_mode', $env->getMode());
$this->assertEquals('global_relay', $env->getRelayAddress());
$this->assertEquals('global_version', $env->getVersion());
$this->assertEquals('tcp://127.0.0.1:6001', $env->getRPCAddress());
}
}
Loading