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

Move CustomFasSyncInterval to Beta #2646

Closed
markmandel opened this issue Jun 27, 2022 · 8 comments · Fixed by #2654
Closed

Move CustomFasSyncInterval to Beta #2646

markmandel opened this issue Jun 27, 2022 · 8 comments · Fixed by #2654
Assignees
Labels
area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc good first issue These are great first issues. If you are looking for a place to start, start here! help wanted We would love help on these issues. Please come help us! kind/feature New features for Agones
Milestone

Comments

@markmandel
Copy link
Member

markmandel commented Jun 27, 2022

Is your feature request related to a problem? Please describe.
The "Custom resync period for FleetAutoscaler" feature for the Fleet Autoscaler has been in Alpha since 1.17.0.

It did have some initial stability issues, but have since been rectified and has been stable in e2e tests since then.

Describe the solution you'd like

Seems like an appropriate time to move to Beta to get some more testing on the feature before moving it to Stable.

Describe alternatives you've considered

Remove the functionality, but it seems useful, so I suggest keeping it.

Additional context

Original issue: #1955
Reference docs:

@markmandel markmandel added kind/feature New features for Agones help wanted We would love help on these issues. Please come help us! good first issue These are great first issues. If you are looking for a place to start, start here! area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc labels Jun 27, 2022
@markmandel
Copy link
Member Author

Checking with everyone related to the original issue - see how the feature has been going for you? @RY-2718 , @jie-bao, @toVersus?

@jie-bao
Copy link
Contributor

jie-bao commented Jun 28, 2022

The original version in 1.17.0 worked well in my environment. I didn't have a chance to use a later version since I changed my job after that. But I'm glad to see it being moved to Beta if others are happy with it.

@toVersus
Copy link

Sorry, we (@RY-2718 and I) didn't have time to verify this feature yet, but we are happy to try this feature on latest Agones and give you some feedback. I’m asking our internal team to raise the priority to work on this.
This is not a blocker to move this feature to beta if someone already adopted and satisfied with the current implementation.

@govargo
Copy link
Contributor

govargo commented Jun 30, 2022

/assign @govargo
I'd like to work on this.

@RY-2718
Copy link

RY-2718 commented Jul 4, 2022

@markmandel @govargo

Sorry for the late reply, we are working on verifying this feature to give our feedback by the end of July, so it would be appreciated if you could wait for merging the PR (#2654).
We are planning to share our thoughts about the API shape and report back whether the situation is improved by adjusting the resync interval or not.
If we don't give any feedback by the end of July, we don't mind merging the PR (#2654).

@markmandel
Copy link
Member Author

🤔 I'm wondering if we need to block on this by end of July?

Reasons being:

  1. It's a good feature, so I don't see any reason not to include it.
  2. Do we expect the config structure to need to change?

If the resync period works better to your specific use case could be handled totally separately, as if it didn't, we'd have to include a different feature as anyway.

WDYT?

@toVersus
Copy link

toVersus commented Jul 7, 2022

If the resync period works better to your specific use case could be handled totally separately, as if it didn't, we'd have to include a different feature as anyway.

It totally makes sense. Sorry, we shouldn’t have suggested to block the merging process.
We were wondering if it would be fine to forward the feature stage to beta even if this feature couldn’t resolve the issue we initially reported.
But in response to your comment, we agree that this configuration knob is useful for other users regardless of the result of our verification.

@markmandel
Copy link
Member Author

It totally makes sense. Sorry, we shouldn’t have suggested to block the merging process.

All good! Appreciate your willingness to take it for a spin, and setting expectations appropriately! 😄

@SaitejaTamma SaitejaTamma added this to the 1.25.0 milestone Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc good first issue These are great first issues. If you are looking for a place to start, start here! help wanted We would love help on these issues. Please come help us! kind/feature New features for Agones
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants