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

fix services.exec_timeout schema definition #1982

Merged
merged 9 commits into from
Jul 27, 2024

Conversation

gam6itko
Copy link
Contributor

@gam6itko gam6itko commented Jul 26, 2024

Reason for This PR

I've found problem in json schema description of services.exec_timeout
IDE phpStorm recognize 10h as invalid value. But this value present in example.

Discord thread
https://discord.com/channels/538114875570913290/538114876497723392/1265365423965081631

Summary by CodeRabbit

  • New Features

    • Improved documentation for time-related configuration parameters, allowing users to specify complex duration formats such as "2h2m1s."
    • Enhanced schema validation for the "Duration" property to support decimal values and multiple time units, providing clearer guidance with additional examples.
  • Documentation

    • Updated command line examples in documentation to improve clarity by removing unnecessary characters.

Copy link
Contributor

coderabbitai bot commented Jul 26, 2024

Walkthrough

The recent changes enhance the documentation and validation for time duration values in configuration files. Notably, the regex patterns for duration formats have been refined to accommodate more flexible and precise specifications, allowing for complex combinations of time units. Additionally, documentation updates clarify these changes, ensuring users have better guidance when configuring time-related parameters.

Changes

Files Change Summary
.rr.yaml, schemas/config/2.0.schema.json, schemas/config/3.0.schema.json Updated duration comments and regex patterns to allow complex time formats; added examples like "2.5h" and "1h3m40s500ms."
schemas/readme.md Removed leading dollar sign from command example for clarity.

Poem

🐇 In the garden, time takes flight,
With hours and minutes, shining bright!
“2.5h” now dances in the air,
Valid durations, beyond compare.
Hopping along with joy we sing,
For clearer paths that changes bring! 🌼✨

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?

Share

@rustatian rustatian self-requested a review July 26, 2024 10:29
@rustatian rustatian added the B-bug Bug: bug, exception label Jul 26, 2024
@rustatian rustatian added this to the v2024.2.1 milestone Jul 26, 2024
Copy link

codecov bot commented Jul 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 35.55%. Comparing base (d7f6667) to head (e169361).
Report is 133 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1982      +/-   ##
==========================================
- Coverage   37.29%   35.55%   -1.74%     
==========================================
  Files          16       17       +1     
  Lines         866      779      -87     
==========================================
- Hits          323      277      -46     
+ Misses        517      471      -46     
- Partials       26       31       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 9f770b1 and 6011cd2.

Files selected for processing (2)
  • schemas/config/2.0.schema.json (1 hunks)
  • schemas/config/3.0.schema.json (1 hunks)
Additional comments not posted (4)
schemas/config/2.0.schema.json (2)

2235-2235: Improved regex pattern for duration validation.

The updated regex pattern ^[0-9]+(h|m|s|ms)$ is more precise and ensures that only valid duration formats are accepted. This change improves validation accuracy.


2237-2237: Added example for duration format.

The addition of the example "9999h" provides clarity on valid duration formats and helps users understand the expected input.

schemas/config/3.0.schema.json (2)

2362-2362: LGTM! The updated pattern improves validation accuracy.

The new pattern ^[0-9]+(h|m|s|ms)$ ensures stricter validation for the Duration field.


2364-2364: LGTM! The added example enhances clarity.

Including "9999h" in the examples helps users understand the expected format for the Duration field.

@gam6itko gam6itko marked this pull request as draft July 26, 2024 10:52
@gam6itko gam6itko marked this pull request as ready for review July 26, 2024 20:17
@gam6itko
Copy link
Contributor Author

supported formats

package main

import (
	"fmt"
	"time"
)

func main() {
	var d time.Duration
	d, _ = time.ParseDuration("1h") //1h0m0s
	fmt.Println(d)
	d, _ = time.ParseDuration("2.5h") //2h30m0s
	fmt.Println(d)
	d, _ = time.ParseDuration("2m") //2m0s
	fmt.Println(d)
	d, _ = time.ParseDuration(".2m") //12s
	fmt.Println(d)
	d, _ = time.ParseDuration("30s") //30s
	fmt.Println(d)
	d, _ = time.ParseDuration("30.03s") //30.03s
	fmt.Println(d)
	d, _ = time.ParseDuration("300ms") //300ms
	fmt.Println(d)
	d, _ = time.ParseDuration("1h3m40s500ms") //1h3m40.5s
	fmt.Println(d)
}

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6011cd2 and e169361.

Files selected for processing (4)
  • .rr.yaml (5 hunks)
  • schemas/config/2.0.schema.json (1 hunks)
  • schemas/config/3.0.schema.json (1 hunks)
  • schemas/readme.md (1 hunks)
Files skipped from review due to trivial changes (2)
  • .rr.yaml
  • schemas/readme.md
Files skipped from review as they are similar to previous changes (2)
  • schemas/config/2.0.schema.json
  • schemas/config/3.0.schema.json

@rustatian
Copy link
Member

LGTM, thanks @gam6itko 👍

@rustatian rustatian merged commit 8375d31 into roadrunner-server:master Jul 27, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-bug Bug: bug, exception
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants