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

remove(api): Versions 1 to 2 are Gone or Not Found #667

Merged
merged 6 commits into from May 8, 2023
Merged

Conversation

Flix6x
Copy link
Contributor

@Flix6x Flix6x commented May 5, 2023

Remove API versions 1.0, 1.1, 1.2, 1.3 and 2.0, while allowing hosts to switch between HTTP status 410 (Gone) and HTTP status 404 (Not Found) responses.

Follow-up issue: remove old data classes.

Flix6x added 4 commits May 5, 2023 20:21
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x self-assigned this May 5, 2023
@Flix6x Flix6x added this to In progress in Deprecate old database models via automation May 5, 2023
@Flix6x Flix6x changed the title Remove api 1 to 2 remove(api): Versions 1 to 2 are Gone or Not Found May 5, 2023
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x marked this pull request as ready for review May 5, 2023 19:01
@Flix6x Flix6x requested a review from nhoening May 5, 2023 19:04
@nhoening
Copy link
Contributor

nhoening commented May 5, 2023

For how long do you plan to allow users to return 410 for these routes - one more version?

@Flix6x
Copy link
Contributor Author

Flix6x commented May 6, 2023

For how long do you plan to allow users to return 410 for these routes - one more version?

Yes, one more. So we'd have this sequence:

  1. 200 with toggle to 410 (blackout test) and relevant headers
  2. 410 with toggle to 200 (sunset rollback)
  3. 404 with toggle to 410 (removal rollback)
  4. 404 (routes removed)

I could ask in the TSC whether stage 3 is redundant.

@nhoening
Copy link
Contributor

nhoening commented May 6, 2023

I think this way to explain our policy would be great on https://flexmeasures.readthedocs.io/en/latest/api/introduction.html#deprecation-and-sunset

Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

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

I approve, if the discussed addition ti the docs happens: document the steps of an deprecation/sunset/gone cycle

Deprecate old database models automation moved this from In progress to Reviewer approved May 8, 2023
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x
Copy link
Contributor Author

Flix6x commented May 8, 2023

@nhoening Maybe do a quick check on my documentation changes?

@Flix6x Flix6x requested a review from nhoening May 8, 2023 11:55
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

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

I like it, thanks!

  • In my opinion, stage 3 is maybe not needed and overcomplicates things a bit. Not sure when and how we should make this decision. We can ask in the TAC. Surely before 0.14.
  • Do we already implement support for these stages in the flexmeasures_client?

@Flix6x
Copy link
Contributor Author

Flix6x commented May 8, 2023

  • I agree to elevate the discussion. In the LF Energy TAC, or FlexMeasures TSC? I was thinking the latter.
  • The client will check for relevant headers and log warnings, and will raise exceptions for 4xx status codes.

@Flix6x Flix6x merged commit a746ae4 into main May 8, 2023
7 checks passed
Deprecate old database models automation moved this from Reviewer approved to Done May 8, 2023
@Flix6x Flix6x deleted the remove-api-1-to-2 branch May 8, 2023 13:15
@nhoening
Copy link
Contributor

nhoening commented May 8, 2023

Yes, TSC

@nhoening nhoening added this to the 0.14.0 milestone May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants