Skip to content

Conversation

@stefangusa
Copy link
Contributor

Bug which alters the normal functioning of the sensor.

@stefangusa stefangusa force-pushed the patch-2 branch 2 times, most recently from 39bb3ab to 1df93b9 Compare May 17, 2020 09:55
Bug which alters the normal functioning of the sensor.
@nmaludy
Copy link
Contributor

nmaludy commented May 17, 2020

Can you show an example of how this is broken?

I'm sure someone put in the urlunparse for a reason.

@stefangusa
Copy link
Contributor Author

@nmaludy Let me make a quick overview:

In #87 , I have added the multiaccount SQS sensor capability which allowed the usage of AWS SQS instances URL, along with the name. I have also implemented the unit tests, which now I change.

I have used my fork implementing that until the PR was merged. On my fork, I used to have a more naïve parsing of the URL, and here, the queue parameter used to be a string.

When reviewing, there was a suggestion to use six library to parse the URL and I had to change also the unit tests before merging, this is from where the urlparse appeared in the unit tests. The queue parameter here became a six object.

However, now when I tried to use the official setup, the trigger - which is mocked in the unit tests - cannot be triggered as the queue must be a string. So this is what I am trying to change now. The unit tests must be changed as a consequence.

I have already tested the change and works perfectly.

Copy link
Contributor

@nmaludy nmaludy left a comment

Choose a reason for hiding this comment

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

Sounds good, you just need to bump the pack version in pack.yaml to 1.3.4

@nmaludy nmaludy merged commit 6d2fdb0 into StackStorm-Exchange:master May 18, 2020
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