Skip to content

Conversation

@yim-lee
Copy link
Member

@yim-lee yim-lee commented Jul 22, 2022

Part of #999


public static func leadershipChange(_ leadershipChange: LeadershipChange) -> Event {
.init(.leadershipChange(leadershipChange))
}
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm... so this one I'd kind of want to keep as an enum to be honest :\

The primary way to use it is to switch over it in an async for loop of events...

Perhaps we should do this case _PLEASE_DO_NOT_EXHAUSTIVELY_MATCH_THIS_ENUM_NEW_CASES_MIGHT_BE_ADDED_IN_THE_FUTURE trick for this one specifically... other SSWG libs do this for such cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

amended 7ea7b1f

Internally we just log error and/or ignore the "fake" event. Is that ok? I don't think we should do default because we will forget to update the switch when we add new event in the future. It doesn't feel right to throw error or crash either.

Copy link
Member

@ktoso ktoso Jul 25, 2022

Choose a reason for hiding this comment

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

Agreed, that looks good 👍


extension OpLog.SequencedOp.SequenceRange: Codable {
public enum DiscriminatorKeys: String, Codable {
enum DiscriminatorKeys: String, Codable {
Copy link
Member

Choose a reason for hiding this comment

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

yep good to keep internal!

/// Upon noticing that this member is marked as [.down], initiate a shutdown.
public static func gracefulShutdown(delay: Duration) -> OnDownActionStrategySettings {
.init(.gracefulShutdown(delay: delay))
}
Copy link
Member

Choose a reason for hiding this comment

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

good one, thank you -- yes this is a good candidate for this treatment, LGTM

@ktoso
Copy link
Member

ktoso commented Jul 25, 2022

Downing and OpLog LGTM, the Event I guess we may need to do the terrible approach instead... :-/
One day we'll get better enums in swift, like the stdlib has... but until then we have the hack heh :\

@ktoso
Copy link
Member

ktoso commented Jul 25, 2022

Flaky #971

@ktoso ktoso merged commit ad50c0c into apple:main Jul 25, 2022
@yim-lee yim-lee deleted the iss999-1 branch July 26, 2022 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants