Skip to content
This repository has been archived by the owner on Dec 4, 2019. It is now read-only.

Fix: Fix issue sending messages too big for Azure Service Bus #31

Merged
merged 1 commit into from
Oct 24, 2017

Conversation

sarvaje
Copy link
Contributor

@sarvaje sarvaje commented Oct 21, 2017

If a message is too big, we change the messages in the rules with error to a generic one.
Also, if a worker fail, we continue updating the other rules in other workers.

Fix #29

@sarvaje sarvaje requested review from molant and alrra October 21, 2017 00:14
@qzhou1607-zz
Copy link

@sarvaje Just to make sure: the job status is changed to error after updating all the results of the other rules, right? Currently in the front end, as soon as the job status is changed to error, all pending rules are marked as scan failed. Let me know if the logic there needs to be changed to reflect the additional rules that didn't fail.

@sarvaje
Copy link
Contributor Author

sarvaje commented Oct 23, 2017

@qzhou1607 I will take a look to sonarwhal.com later to take into account these changes.

elementLine: -1,
line: -1
},
message: 'Error in sonar analyzing this rule',
Copy link
Member

Choose a reason for hiding this comment

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

Maybe say the name of the rule in the message?

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 is the message we are going to see when we expand the rule, so I'm not sure if it is necessary to show the name or not.

I don't have a strong opinion on it, so I can change it if you want to :)

Copy link
Member

Choose a reason for hiding this comment

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

OK, I thought it was for logging. OK as it is

elementLine: -1,
line: -1
},
message: 'This rule has a lot errors, please use Sonar in your local machine for more errors details',
Copy link
Member

Choose a reason for hiding this comment

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

This rule has too many errors. Please use sonar locally for more details


return;
}

if (job.status === JobStatus.started) {
// When the a job is splitted we receive more than one messges for the status `started`
Copy link
Member

Choose a reason for hiding this comment

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

When a job is split we receive more than one message for the status started

If a message is too big, we change the messages in the rules with error to a generic one.
Also, if a worker fail, we continue updating the other rules in other workers.

Fix webhintio#29
@molant molant merged commit 812400b into webhintio:master Oct 24, 2017
@molant
Copy link
Member

molant commented Oct 24, 2017

@sarvaje #31 has been merged. Feel free to merge this once the changes are deployed.

@sarvaje
Copy link
Contributor Author

sarvaje commented Oct 24, 2017

@molant you mean that I can merge the PR in sonarwhal.com, right? webhintio/webhintio.github.io#260

@molant
Copy link
Member

molant commented Oct 24, 2017

Yes

@sarvaje
Copy link
Contributor Author

sarvaje commented Oct 24, 2017

Done!

@molant molant mentioned this pull request Oct 26, 2017
2 tasks
@sarvaje sarvaje deleted the split-messages branch May 15, 2018 20:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants