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

Rename args in some API endpoints to be more consistent #8853

Merged
merged 5 commits into from
Jun 3, 2024

Conversation

kiendang
Copy link
Contributor

@kiendang kiendang commented May 24, 2024

See #8802 (comment)

  • Rename arg image to image_uid in worker_image_service.push to be consistent with other endpoints.
  • Rename arg pool to pool_name in worker_pool_service.launch `to be consistent with other endpoints.
  • Rename arg pull in worker_image_service.build to be consistent with worker_pool_service.create_image_and_pool_request(pull_image: bool)
  • Rename reg_username and reg_password to registry_username and registry_password. The later are much clearer while not much longer. Plus we are using registry_uid anyway.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@kiendang kiendang force-pushed the cleanup-api-args branch 6 times, most recently from 64ab723 to dc6d4cc Compare May 30, 2024 07:39
@kiendang kiendang marked this pull request as ready for review May 30, 2024 07:39
@kiendang kiendang requested a review from shubham3121 May 30, 2024 11:46
to be consistent with other endpoints
to be consistent with other endpoints
to be consistent with worker_pool_service.create_image_and_pool_request
Copy link
Member

@shubham3121 shubham3121 left a comment

Choose a reason for hiding this comment

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

LGTM !! 💯 🎉

@shubham3121 shubham3121 merged commit 842fde9 into OpenMined:dev Jun 3, 2024
23 checks passed
@kiendang kiendang deleted the cleanup-api-args branch June 3, 2024 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants