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

Add silent message update support #287

Merged

Conversation

yinan-symphony
Copy link
Contributor

  • Update Symphony APIs spec version

  • Add silent flag support according to the new spec.
    When silent is true, the new updated message is marked as read, otherwise is unread.
    True is the default value.

Description

Closes #[ISSUE NUMBER]

Please put here the intent of your pull request.

Dependencies

List the other pull requests that should be merged before/along this one.

Checklist

  • Referenced an issue in the PR title or description
  • Filled properly the description and dependencies, if any
  • Unit tests updated or added
  • Docstrings added or updated
  • Updated the documentation in docsrc folder

@@ -4,7 +4,7 @@ code_gen_dir=`pwd`
project_root=$code_gen_dir/..
echo $code_gen_dir

commit_hash=136b530512eea112de73591906051871d034750a
Copy link
Contributor

Choose a reason for hiding this comment

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

When we update this commit hash, we might want to run the sh script to update the generated code. See here https://github.com/finos/symphony-bdk-python/blob/main/docsrc/markdown/tech/generated_api.md

Copy link
Contributor

Choose a reason for hiding this comment

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

And include the new generated files in the PR of course.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, i am doing it now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -466,7 +466,7 @@ async def search_messages_one_page(skip, limit):

@retry
async def update_message(self, stream_id: str, message_id: str, message: Union[str, Message], data=None,
version: str = "") -> V4Message:
silent=True, version: str = "") -> V4Message:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if adding this parameter doesn't break the backward compatibility. I would rather add a new method that takes this new argument, and I call it in this one with the default silent value (True).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i checked this point actually. this silent=True syntax in python is working as an optional arg with the default value. so user can choose to specify it or simply ignore it, in this case the default value is applied.
you can see that the unit test does not change the signature, but it is still working.

@yinan-symphony yinan-symphony force-pushed the feature/silent-message-update branch from e74e31c to d07b36e Compare May 25, 2022 13:44
 - Update Symphony APIs spec version

 - Add silent flag support according to the new spec.
   When silent is true, the new updated message is marked as read, otherwise is unread.
   True is the default value.
@yinan-symphony yinan-symphony force-pushed the feature/silent-message-update branch from d07b36e to 8135810 Compare May 25, 2022 15:13
@symphony-soufiane symphony-soufiane merged commit 13f9948 into finos:main May 31, 2022
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