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

API gateway versioning #354

Merged
merged 24 commits into from
Apr 3, 2023
Merged

API gateway versioning #354

merged 24 commits into from
Apr 3, 2023

Conversation

Tansito
Copy link
Member

@Tansito Tansito commented Mar 24, 2023

Summary

This PR includes support in gateway api for different versions and introduce the change in the client.

Details and comments

  • Applied versioning structure to support different versions in the API
  • Modified the client to start using the versions
  • ViewSets to support different versions now has dynamic serializers with get_serializer_class
  • Refactorization of ray_job_status_to_model_job_status
  • Refactorization of User model inside job to remove the pylint error, now we access the user through get_user_model
  • Updated the tests accordingly
  • Refactorization of run_program end-point to be just run
  • requests package is added to requirements

Fix #308

@Tansito Tansito marked this pull request as ready for review March 24, 2023 16:58
@Tansito Tansito changed the title 🏗️ API gateway versioning API gateway versioning Mar 24, 2023
Copy link
Collaborator

@akihikokuroda akihikokuroda left a comment

Choose a reason for hiding this comment

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

LGTM!!

Copy link
Collaborator

@psschwei psschwei left a comment

Choose a reason for hiding this comment

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

minor nit: in one place we use if version is None whereas in the other we use version = version or os.environ... do we want to standardize?

@Tansito
Copy link
Member Author

Tansito commented Mar 30, 2023

I couldn't standardize them because the use-case is different. In the first scenario I'm receiving a version from the constructor with an standard value but in the second one no, it's a simple function.

My idea is to refactorize save_result inside Job and in that moment I think I'm going to be able to make them equals. But I need to review if I can do that change first and I was trying to avoid to make that in this PR.

Forget it, you are right in fact there was a bug in the logic 😂

@Tansito
Copy link
Member Author

Tansito commented Mar 30, 2023

Now it should work as expected @psschwei . If a user specifies a version overrides everything, if not it goes the environment and at the end the default value.

I can't add the version from the user in the second scenario because save_result is outside Job but the logic is similar. My idea is to refactorize that method later in another PR including it in the Job class. I think it has sense but I need to review it well in case there was a reason because we had it like that.

# Conflicts:
#	gateway/main/urls.py
@Tansito
Copy link
Member Author

Tansito commented Apr 3, 2023

From what I tested there is no breaking changes in the documentation plus all the changes were internal so merging it right now!

@Tansito Tansito merged commit 0d7c3c2 into main Apr 3, 2023
@Tansito Tansito deleted the gateway-versioning branch April 3, 2023 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement versioning for the end-points
4 participants