Skip to content

Firebase ML Kit Publish and Unpublish Implementation #345

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

Merged
merged 4 commits into from
Sep 11, 2019
Merged

Conversation

ifielker
Copy link
Contributor

Firebase ML Kit Publish and Unpublish Implementation

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

A few renaming suggestions.

@@ -659,6 +686,17 @@ def update_model(self, model, update_mask=None):
except requests.exceptions.RequestException as error:
raise _utils.handle_platform_error_from_requests(error)

def publish_model(self, model_id, publish):
Copy link
Contributor

Choose a reason for hiding this comment

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

May be call this set_published()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

(mlkit.publish_model, True),
(mlkit.unpublish_model, False)
]
PUBLISH_AND_UNPUBLISH_ARGS = [mlkit.publish_model, mlkit.unpublish_model]
Copy link
Contributor

Choose a reason for hiding this comment

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

PUBLISH_AND_UNPUBLISH_FUNCS = [item[0] for item in PUBLISH_AND_UNPUBLISH_FUNCS_WITH_ARGS]

Copy link
Contributor Author

@ifielker ifielker Sep 11, 2019

Choose a reason for hiding this comment

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

done

@@ -277,7 +277,11 @@
PAGE_SIZE_VALUE_ERROR_MSG = 'Page size must be a positive integer between ' \
'1 and {0}'.format(mlkit._MAX_PAGE_SIZE)
INVALID_STRING_OR_NONE_ARGS = [0, -1, 4.2, 0x10, False, list(), dict()]

PUBLISH_AND_UNPUBLISH_VALUE_ARGS = [
Copy link
Contributor

Choose a reason for hiding this comment

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

PUBLISH_AND_UNPUBLISH_FUNCS_WITH_ARGS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pylint doesn't like more than 30 characters. Changed to PUBLISH_UNPUBLISH_WITH_ARGS

@@ -277,7 +277,11 @@
PAGE_SIZE_VALUE_ERROR_MSG = 'Page size must be a positive integer between ' \
'1 and {0}'.format(mlkit._MAX_PAGE_SIZE)
INVALID_STRING_OR_NONE_ARGS = [0, -1, 4.2, 0x10, False, list(), dict()]

PUBLISH_AND_UNPUBLISH_VALUE_ARGS = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of putting these in the module namespace, you might want to put them in the TestPublishUnpublish class.

Same goes for most other constants in this file. But you can do that in a future PR if you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done for these 2. I can move the rest in a future PR

@hiranya911 hiranya911 assigned ifielker and unassigned hiranya911 Sep 11, 2019
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

LGTM

@hiranya911 hiranya911 removed their assignment Sep 11, 2019
@ifielker ifielker merged commit 0344172 into mlkit Sep 11, 2019
@ifielker ifielker deleted the mlkit-8 branch September 11, 2019 23:55
lahirumaramba pushed a commit that referenced this pull request Apr 20, 2020
* Introduced the exceptions module (#296)

* Added the exceptions module

* Cleaned up the error handling logic; Added tests

* Updated docs; Fixed some typos

* Migrating FCM Send APIs to the New Exceptions (#297)

* Migrated FCM send APIs to the new error handling regime

* Moved error parsing logic to _utils

* Refactored OP error handling code

* Fixing a broken test

* Added utils for handling googleapiclient errors

* Added tests for new error handling logic

* Updated public API docs

* Fixing test for python3

* Cleaning up the error code lookup code

* Cleaning up the error parsing APIs

* Cleaned up error parsing logic; Updated docs

* Migrated remaining messaging APIs to new error types (#298)

* Migrated FCM send APIs to the new error handling regime

* Moved error parsing logic to _utils

* Refactored OP error handling code

* Fixing a broken test

* Added utils for handling googleapiclient errors

* Added tests for new error handling logic

* Updated public API docs

* Fixing test for python3

* Cleaning up the error code lookup code

* Cleaning up the error parsing APIs

* Cleaned up error parsing logic; Updated docs

* Migrated the FCM IID APIs to the new error types

* Introducing TokenSignError to represent custom token creation errors (#302)

* Migrated FCM send APIs to the new error handling regime

* Moved error parsing logic to _utils

* Refactored OP error handling code

* Fixing a broken test

* Added utils for handling googleapiclient errors

* Added tests for new error handling logic

* Updated public API docs

* Fixing test for python3

* Cleaning up the error code lookup code

* Cleaning up the error parsing APIs

* Cleaned up error parsing logic; Updated docs

* Migrated the FCM IID APIs to the new error types

* Migrated custom token API to new error types

* Raising FirebaseError from create_session_cookie() API (#306)

* Migrated FCM send APIs to the new error handling regime

* Moved error parsing logic to _utils

* Refactored OP error handling code

* Fixing a broken test

* Added utils for handling googleapiclient errors

* Added tests for new error handling logic

* Updated public API docs

* Fixing test for python3

* Cleaning up the error code lookup code

* Cleaning up the error parsing APIs

* Cleaned up error parsing logic; Updated docs

* Migrated the FCM IID APIs to the new error types

* Migrated custom token API to new error types

* Migrated create cookie API to new error types

* Improved error message computation

* Refactored the shared error handling code

* Fixing lint errors

* Renamed variable for clarity

* Introducing UserNotFoundError type (#309)

* Added UserNotFoundError type

* Fixed some lint errors

* Some formatting updates

* Updated docs and tests

* New error handling support in create/update/delete user APIs (#311)

* New error handling support in create/update/delete user APIs

* Fixing some lint errors

* Error handling improvements in email action link APIs (#312)

* New error handling support in create/update/delete user APIs

* Fixing some lint errors

* Error handling update in email action link APIs

* Project management API migrated to new error types (#314)

* Error handling updated for remaining user_mgt APIs (#315)

* Error handling updated for remaining user_mgt APIs

* Removed unused constants

* Migrated token verification APIs to new exception types (#317)

* Migrated token verification APIs to new error types

* Removed old AuthError type

* Added new exception types for revoked tokens

* Migrated the db module to the new exception types (#318)

* Migrating db module to new exception types

* Error handling for transactions

* Updated integration tests

* Restoring the old txn abort behavior

* Updated error type in snippet

* Added comment

* Adding a few overlooked error types (#319)

* Adding some missing error types

* Updated documentation

* Removing the ability to delete user properties by passing None (#320)

* Adding beginning of _MLKitService (#323)

* Adding beginning of _MLKitService

* Added License and Docstring

* Firebase ML Kit Get Model API implementation (#326)

* added GetModel
* Added tests for get_model

* Firebase ML Kit Delete Model API implementation (#327)

* implement delete model

* Firebase ML Kit List Models API implementation (#331)

* implemented list models plus tests

* Implementation of Model, ModelFormat, TFLiteModelSource and subclasses (#335)

* Implementation of Model, ModelFormat, ModelSource and subclasses

* Firebase ML Kit Create Model API implementation (#337)

* create model plus long running operation handling
* Model.wait_for_unlocked

* Firebase ML Kit Update Model API implementation (#343)

* Firebase ML Kit Create Model API implementation

* Firebase ML Kit Publish and Unpublish Implementation (#345)

* Firebase ML Kit Publish and Unpublish Implementation

* Firebase ML Kit TFLiteGCSModelSource.from_tflite_model implementation and conversion helpers (#346)

* Firebase ML Kit TFLiteGCSModelSource.from_tflite_model implementation
* support for tensorflow lite conversion helpers (version 1.x)

* Quick pass at filling in missing docstrings (#367)

* Quick pass at filling in missing docstrings

* More punctuation

* Modify Operation Handling to not require a name for Done Operations (#371)

* Firebase ML Kit Modify Operation Handling to not require a name for Done Operations
* Adding support for TensorFlow 2.x

* rename from mlkit to ml (#373)

* Adding File naming capability to from_saved_model and from_keras_model. (#375)

adding File naming capability for ModelSource

* Firebase ML Modify Operation Handling Code to match rpc codes not html codes (#390)

* Firebase ML Modify Operation Handling Code to match actual codes
* apply database fix too

* Mlkit fix date handling2 (#391)

* Fix create/update date handling
* Skip unrelated failing tests (until sync)

* Firebase Ml Fix upload file naming (#392)

* Fix File Naming

* Integration tests for Firebase ML (#394)

* Integration tests for Firebase ML

* Fixing lint errors for Py3 (#401)

* Fixing lint errors for Py3

* Removed dependency on six

* Fixing a couple of merge errors

* Modifying operation handling to support backend changes (#423)

* modifying operation handling to support backend changes

* Firebase ML Changing service endpoint (#421)

* Mlkit add headers (#445)

* add Headers

* fixed test (#448)

* Adding tensorflow and keras so we don't skip tests (#449)

* Adding tensorflow and keras so we don't skip tests
* Add additional instructions for integration tests for ml

Co-authored-by: Hiranya Jayathilaka <[email protected]>
Co-authored-by: Kevin Cheung <[email protected]>
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.

2 participants