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

LITE-28666 search by parameter value was fixed #91

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

zzzevaka
Copy link
Contributor

  • search by parameter value was fixed
  • ignore FF requests without changes in case of duplications

if not attempts_left:
raise
continue
result = await self.retrieve_ff_requests(lookup)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please clarify why you decided to remove retries?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This timeout was needed because the Connect client wasn't retrying in case of connection errors.

It was fixed cloudblue/connect-python-openapi-client#74

What I forgot to do is bump the requirements to apply this patch.

This retry should be also deleted in subscriptions lookup. I will do it a bit later.

for request in requests:
changed = False
for item_data in request['asset']['items']:
if item_data['quantity'] != item_data['old_quantity']:
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 not sure that this will work for 100% of scenarios. Can it be 2 request found and both are SYNC? If yes, we will get an empty result.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is theoretically possible. Thanks!

@bdjilka
Copy link
Contributor

bdjilka commented Sep 21, 2023

Have just 2 questions. In general everything is good. Thank for adding old_quantity item field, it will be helpful

ignore FF requests without changes in case of duplications
ignore FF requests that was approved after batch.context.period.end
@zzzevaka zzzevaka force-pushed the feature/LITE-28666_ff-lookup branch from becd350 to 17051ec Compare September 25, 2023 13:26
@bdjilka bdjilka merged commit 0ef2ced into master Sep 25, 2023
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.

3 participants