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

eskip: remove flowid dependency #3065

Merged
merged 1 commit into from
May 8, 2024

Conversation

AlexanderYastrebov
Copy link
Member

eskip package is generally useful without proxy or filters and therefore ideally should have minimal number of dependencies.

This change deprecates GenerateIfNeeded which is only used by etcd dataclient and reimplements it to remove dependency on github.com/zalando/skipper/filters/flowid

See 35ba6cc

@AlexanderYastrebov AlexanderYastrebov added the minor no risk changes, for example new filters label May 6, 2024
eskip package is generally useful without proxy or filters and therefore
ideally should have minimal number of dependencies.

This change deprecates GenerateIfNeeded which is only used by etcd dataclient and
reimplements it to remove dependency on github.com/zalando/skipper/filters/flowid

See 35ba6cc

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>

// generate weak random id for a route if
// it doesn't have one.
//
// Deprecated: do not use, generate valid route id that matches [a-zA-Z_] yourself.
Copy link
Member

Choose a reason for hiding this comment

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

deprecate in the sense of stable?
If so, why do we need to deprecate it?
If we do not plan to delete it, then it's just fine and we do not need to apply the ignore SA1019

Copy link
Member Author

Choose a reason for hiding this comment

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

To hint lib users to avoid it.
I'd also delete it in the future as it has strange API contract.

Or do you propose that we keep it or replace with something else?

Copy link
Member

Choose a reason for hiding this comment

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

I just wanted to ask what the plan is, because the PR description nor commit does show what the plan is.
And you talked about deprecated is good because it means it is stable in the last days.

@szuecs
Copy link
Member

szuecs commented May 8, 2024

👍

1 similar comment
@AlexanderYastrebov
Copy link
Member Author

👍

@AlexanderYastrebov AlexanderYastrebov merged commit b778f2b into master May 8, 2024
14 checks passed
@AlexanderYastrebov AlexanderYastrebov deleted the eskip/decouple-from-filters branch May 8, 2024 08:59
JanardhanSharma pushed a commit to JanardhanSharma/skipper that referenced this pull request Jul 19, 2024
eskip package is generally useful without proxy or filters and therefore
ideally should have minimal number of dependencies.

This change deprecates GenerateIfNeeded which is only used by etcd dataclient and
reimplements it to remove dependency on github.com/zalando/skipper/filters/flowid

See 35ba6cc

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor no risk changes, for example new filters
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants