update time library and misc fixes #74
Conversation
WalkthroughThe changes involve modifications to various components of the project, including the CI/CD pipeline configuration, Dockerfile syntax, command-line interface options, configuration handling, and event queue management. Notable updates include the removal of specific performance tests, the introduction of a Typesense URL option in the CLI, and enhancements to time handling in the event queue. Overall, these changes aim to improve functionality and code clarity across the codebase. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Config
participant EventQueue
CLI->>Config: Set typesense-url
Config->>EventQueue: Apply options
EventQueue->>EventQueue: Calculate delay using as(TimeUnit::Seconds)
EventQueue-->>CLI: Return updated configuration
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v2 #74 +/- ##
============================================
- Coverage 44.89% 44.82% -0.07%
Complexity 1098 1098
============================================
Files 94 94
Lines 3212 3219 +7
============================================
+ Hits 1442 1443 +1
- Misses 1770 1776 +6 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- .github/workflows/build-cli.yaml (1 hunks)
- Dockerfile (1 hunks)
- cli/cli.go (1 hunks)
- cli/config/config.go (1 hunks)
- cli/glue/glue.go (2 hunks)
- cli/go.mod (1 hunks)
- cli/lib/consumer.go (1 hunks)
- src/Events/EventQueue.php (3 hunks)
- src/Glue/glue.php (4 hunks)
Files skipped from review due to trivial changes (3)
- .github/workflows/build-cli.yaml
- Dockerfile
- cli/go.mod
Additional context used
GitHub Check: codecov/patch
src/Events/EventQueue.php
[warning] 104-105: src/Events/EventQueue.php#L104-L105
Added lines #L104 - L105 were not covered by tests
[warning] 124-124: src/Events/EventQueue.php#L124
Added line #L124 was not covered by tests
[warning] 129-129: src/Events/EventQueue.php#L129
Added line #L129 was not covered by tests
[warning] 131-131: src/Events/EventQueue.php#L131
Added line #L131 was not covered by testssrc/Glue/glue.php
[warning] 89-89: src/Glue/glue.php#L89
Added line #L89 was not covered by tests
[warning] 96-98: src/Glue/glue.php#L96-L98
Added lines #L96 - L98 were not covered by tests
[warning] 103-103: src/Glue/glue.php#L103
Added line #L103 was not covered by tests
[warning] 193-198: src/Glue/glue.php#L193-L198
Added lines #L193 - L198 were not covered by tests
[warning] 205-206: src/Glue/glue.php#L205-L206
Added lines #L205 - L206 were not covered by tests
[warning] 314-314: src/Glue/glue.php#L314
Added line #L314 was not covered by tests
[warning] 318-318: src/Glue/glue.php#L318
Added line #L318 was not covered by tests
[warning] 322-323: src/Glue/glue.php#L322-L323
Added lines #L322 - L323 were not covered by tests
Additional comments not posted (8)
cli/config/config.go (1)
117-119: LGTM!The new option
typesense-urlenhances the configurability of theConfigobject and is correctly implemented.The code changes are approved.
src/Events/EventQueue.php (2)
104-105: LGTM!The change to use
as(TimeUnit::Seconds)enhances clarity and improves type safety.The code changes are approved.
Tools
GitHub Check: codecov/patch
[warning] 104-105: src/Events/EventQueue.php#L104-L105
Added lines #L104 - L105 were not covered by tests
124-131: LGTM!The change to use
Timeas the return type reflects a broader representation of time and aligns with the new type system.The code changes are approved.
Tools
GitHub Check: codecov/patch
[warning] 124-124: src/Events/EventQueue.php#L124
Added line #L124 was not covered by tests
[warning] 129-129: src/Events/EventQueue.php#L129
Added line #L129 was not covered by tests
[warning] 131-131: src/Events/EventQueue.php#L131
Added line #L131 was not covered by testscli/lib/consumer.go (1)
292-292: LGTM!The new entry enhances the functionality of the
processMsgfunction by capturing and storing the remote address of the sender.The code changes are approved.
cli/glue/glue.go (2)
102-103: LGTM!The function correctly extracts the client's IP address and includes it in the
envmap and message headers.The code changes are approved.
107-109: LGTM!The function correctly adds the client's IP address to the message headers.
The code changes are approved.
src/Glue/glue.php (1)
193-199: LGTM!The function correctly handles entity signal events.
The code changes are approved.
Tools
GitHub Check: codecov/patch
[warning] 193-198: src/Glue/glue.php#L193-L198
Added lines #L193 - L198 were not covered by testscli/cli.go (1)
568-568: LGTM!The function correctly adds the new command-line option for Typesense URL.
The code changes are approved.
| try { | ||
| $provenance = json_decode($_SERVER['HTTP_DPHP_PROVENANCE'] ?? 'null', true, 32, JSON_THROW_ON_ERROR); | ||
| if (! $provenance || $provenance === ['userId' => '', 'roles' => null]) { | ||
| if (!$provenance || $provenance === ['userId' => '', 'roles' => null]) { |
There was a problem hiding this comment.
Reminder: Add tests for provenance handling.
The provenance handling logic is not covered by tests.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 89-89: src/Glue/glue.php#L89
Added line #L89 was not covered by tests
| $this->logger->alert( | ||
| 'Failed to capture provenance', | ||
| ['provenance' => $_SERVER['HTTP_DPHP_PROVENANCE'] ?? null], |
There was a problem hiding this comment.
Reminder: Add tests for JSON decoding error handling.
The JSON decoding error handling logic is not covered by tests.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 96-98: src/Glue/glue.php#L96-L98
Added lines #L96 - L98 were not covered by tests
| } | ||
|
|
||
| if (! file_exists($_SERVER['HTTP_DPHP_PAYLOAD'])) { | ||
| if (!file_exists($_SERVER['HTTP_DPHP_PAYLOAD'])) { |
There was a problem hiding this comment.
Reminder: Add tests for payload file existence check.
The payload file existence check logic is not covered by tests.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 103-103: src/Glue/glue.php#L103
Added line #L103 was not covered by tests
| if (!$this->target->toOrchestrationInstance()->executionId) { | ||
| $this->target = StateId::fromInstance( | ||
| new OrchestrationInstance( | ||
| $this->target->toOrchestrationInstance()->instanceId, | ||
| Uuid::uuid7()->toString(), | ||
| ), |
There was a problem hiding this comment.
Reminder: Add tests for orchestration instance creation.
The orchestration instance creation logic is not covered by tests.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 193-198: src/Glue/glue.php#L193-L198
Added lines #L193 - L198 were not covered by tests
| $event = | ||
| WithOrchestration::forInstance($this->target, StartExecution::asParent($input, []/* todo: scheduling */)); |
There was a problem hiding this comment.
Reminder: Add tests for event creation.
The event creation logic is not covered by tests.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 205-206: src/Glue/glue.php#L205-L206
Added lines #L205 - L206 were not covered by tests
| case $attribute->getName() === AllowCreateForRole::class: | ||
| /** @var AllowCreateForRole $attribute */ | ||
| $attribute = $attribute->newInstance(); | ||
| /** @var AllowCreateForRole $attribute */ $attribute = $attribute->newInstance(); |
There was a problem hiding this comment.
Reminder: Add tests for role permissions.
The role permissions logic is not covered by tests.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 314-314: src/Glue/glue.php#L314
Added line #L314 was not covered by tests
| case $attribute->getName() === AllowCreateForUser::class: | ||
| /** @var AllowCreateForUser $attribute */ | ||
| $attribute = $attribute->newInstance(); | ||
| /** @var AllowCreateForUser $attribute */ $attribute = $attribute->newInstance(); |
There was a problem hiding this comment.
Reminder: Add tests for user permissions.
The user permissions logic is not covered by tests.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 318-318: src/Glue/glue.php#L318
Added line #L318 was not covered by tests
| /** @var TimeToLive $attribute */ $attribute = $attribute->newInstance(); | ||
| $permissions['ttl'] = $attribute->timeToLive()->as(TimeUnit::Nanoseconds); |
There was a problem hiding this comment.
Reminder: Add tests for time-to-live attribute.
The time-to-live attribute logic is not covered by tests.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 322-323: src/Glue/glue.php#L322-L323
Added lines #L322 - L323 were not covered by tests
Performance Metrics
|
Summary by CodeRabbit
New Features
--typesense-urlfor specifying the Typesense server URL.Bug Fixes
Documentation