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

Deprecated retryWhen usage substituted #17658

Conversation

sergiusz-n
Copy link
Contributor

A few deprecated (as of v.3.3.x) .retryWhen()usages are updated. . The latter prevents from updating dependancy reactor-core to v.3.4.0.

@ghost ghost added Azure.Core azure-core Cosmos customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Nov 18, 2020
@ghost
Copy link

ghost commented Nov 18, 2020

Thank you for your contribution sergiusz-n! We will review the pull request and get back to you soon.

@ghost
Copy link

ghost commented Nov 18, 2020

CLA assistant check
All CLA requirements met.

@alzimmermsft
Copy link
Member

@srnagar @kushagraThapar could you review this PR, this looks to resolve issue #17685

Copy link
Member

@kushagraThapar kushagraThapar left a comment

Choose a reason for hiding this comment

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

LGTM, however, it would be great if someone else can review the specifics of Retry.withThrowable() and Retry.from()?
@anuchandy @srnagar

@kushagraThapar
Copy link
Member

/azp run java - cosmos - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@srnagar
Copy link
Member

srnagar commented Nov 20, 2020

The changes look good to me. Thank you for your contribution @sergiusz-n. We are about to release the core-amqp library and we want to hold off on merging this change until the release is complete. This change can be included for the subsequent release.

@kushagraThapar can you please check why cosmos live tests are failing? Is this related to the changes in this PR?

@kushagraThapar
Copy link
Member

@srnagar @alzimmermsft - I am seeing this error in the build step ->

##[error]Unhandled: Cannot read property 'major' of null

In the past, I have ran several live tests on open source contributions, but seeing this issue for the first time. Can you guys please check what might be causing this ?
I am not re-running the build to avoid loss of logs.

@alzimmermsft
Copy link
Member

@srnagar @alzimmermsft - I am seeing this error in the build step ->

##[error]Unhandled: Cannot read property 'major' of null

In the past, I have ran several live tests on open source contributions, but seeing this issue for the first time. Can you guys please check what might be causing this ?
I am not re-running the build to avoid loss of logs.

Looked into it an found that a new testing variable was added which wasn't present in the Cosmos testing infrastructure. I've submitted this PR (#17762) to resolve this issue by adding a safe global default in case there is other infrastructure that is also missing the value.

@sergiusz-n sergiusz-n force-pushed the deprecated-reactor-retrywhen-substitute branch from 3ff2a36 to 859a4a9 Compare November 24, 2020 22:14
@sergiusz-n
Copy link
Contributor Author

/azp run java - cosmos - tests

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 17658 in repo Azure/azure-sdk-for-java

@kushagraThapar
Copy link
Member

/azp run java - cosmos - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@aderri
Copy link

aderri commented Nov 24, 2020

Hi Guys,

I'm little bit stuck with this issue. Do you have an idea when it's going to be released please ?

Thanks in advance,
Anass

@kushagraThapar
Copy link
Member

Hi Guys,

I'm little bit stuck with this issue. Do you have an idea when it's going to be released please ?

Thanks in advance,
Anass

The release pipeline is failing because of intermittent connectivity issues, we will merge the PR once all the pipelines finish up.

@aderri
Copy link

aderri commented Nov 26, 2020

Hi Guys,

As the pipeline seems to be ok . Could anyone merge this PR please to release the fix ?

Have a nice day,
Anass.

@aderri
Copy link

aderri commented Nov 27, 2020

Hi Guys,
I'm little bit stuck with this issue. Do you have an idea when it's going to be released please ?
Thanks in advance,
Anass

The release pipeline is failing because of intermittent connectivity issues, we will merge the PR once all the pipelines finish up.

Hi,

Could you please accept MR as everything is green now please ?

Thanks,
Anass.

@kushagraThapar kushagraThapar merged commit 750e80b into Azure:master Nov 30, 2020
@sergiusz-n sergiusz-n deleted the deprecated-reactor-retrywhen-substitute branch November 30, 2020 21:28
@aderri
Copy link

aderri commented Dec 16, 2020

Hello guys,

Are you sure to have released this fix ? Because I'm using the latest version (7.0.0) of Azure SDK for Java that includes the 2.0.0 version of amqp. I tried to find this fix but failed.

Could you help me please ?

Anass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core azure-core Cosmos customer-reported Issues that are reported by GitHub users external to the Azure organization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants