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

bug/2584_service_path_as_http_header_in_notifications #2828

Merged
merged 6 commits into from
Jan 20, 2017

Conversation

kzangeli
Copy link
Member

Fixed issue #2584.

@kzangeli kzangeli added this to the 1.7.0 milestone Jan 20, 2017
@@ -192,6 +192,8 @@ Date: REGEX(.*)
========================================================================================
POST http://127.0.0.1:REGEX(\d+)/notify
Content-Length: 459
Fiware-Servicepath: /
Fiware-Servicepath: /
Copy link
Member

Choose a reason for hiding this comment

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

Duplicated

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in deeabf3

@@ -251,6 +253,8 @@ Date: REGEX(.*)
==================================================================================================
POST http://127.0.0.1:REGEX(\d+)/notify
Content-Length: 459
Fiware-Servicepath: /
Fiware-Servicepath: /
Copy link
Member

Choose a reason for hiding this comment

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

Duplicated

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in deeabf3

@@ -237,6 +237,8 @@ Date: REGEX(.*)
========================================================================================
POST http://127.0.0.1:REGEX(\d+)/notify
Content-Length: 459
Fiware-Servicepath: /
Fiware-Servicepath: /
Copy link
Member

Choose a reason for hiding this comment

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

Duplicated

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in deeabf3

@@ -314,6 +316,8 @@ Date: REGEX(.*)
================================================================================
POST http://127.0.0.1:REGEX(\d+)/notify
Content-Length: 460
Fiware-Servicepath: /
Fiware-Servicepath: /
Copy link
Member

Choose a reason for hiding this comment

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

Duplicated

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in deeabf3

@@ -142,6 +142,8 @@ Date: REGEX(.*)
=============================================================
POST http://127.0.0.1:REGEX(\d+)/notify
Content-Length: 459
Fiware-Servicepath: /
Fiware-Servicepath: /
Copy link
Member

Choose a reason for hiding this comment

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

Duplicated

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in deeabf3

@@ -320,6 +323,7 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36})
=======================================
POST http://127.0.0.1:REGEX(\d+)/notify
Fiware-Servicepath: /
Fiware-Servicepath: /
Copy link
Member

Choose a reason for hiding this comment

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

Dup

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in deeabf3

@@ -346,6 +350,7 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36})
PUT http://127.0.0.1:REGEX(\d+)/notify?a1=true&type=T1&id=E1
Fiware-Servicepath: /
Entity-Id: E1
Fiware-Servicepath: /
Copy link
Member

Choose a reason for hiding this comment

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

Dup

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in deeabf3

Copy link
Member

Choose a reason for hiding this comment

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

This is an example of dup with an already existing "Fiware-Servicpath: /" line

@@ -365,6 +370,7 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36})
PUT http://127.0.0.1:REGEX(\d+)/notify?a2=null&type=T2&id=E1
Fiware-Servicepath: /
Entity-Id: E1
Fiware-Servicepath: /
Copy link
Member

Choose a reason for hiding this comment

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

Dup

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in deeabf3

Copy link
Member

Choose a reason for hiding this comment

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

This is an example of dup with an already existing "Fiware-Servicpath: /" line

@@ -385,6 +391,7 @@ PUT http://127.0.0.1:REGEX(\d+)/notify?a3=3&type=T3&id=E1
Fiware-Servicepath: /
Ngsiv2-Attrsformat: custom
Entity-Id: E1
Fiware-Servicepath: /
Copy link
Member

Choose a reason for hiding this comment

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

Dup

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in deeabf3

Copy link
Member

Choose a reason for hiding this comment

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

This is an example of dup with an already existing "Fiware-Servicpath: /" line

@@ -0,0 +1,196 @@
# Copyright 2017 Telefonica Investigacion y Desarrollo, S.A.U
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need a new .test for this when the large ammount of notifications in default SP we have in another tests will ensure the functionality is correct? I does't suggest to remove it (as it is already written) just a remark to think twice next time in similar situations ;)

NTC

Copy link
Member Author

Choose a reason for hiding this comment

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

This is merely a tool that I needed while trying to fix the bug.
And you're right, once the bug is fixed, no need for this test anymore.

And to anticipate :-):
No, it is not easier to try to find a 'half valid' already existing functest than to create a new one.
A new one is both faster and better suited for the problem
To create a functest takes me a few minutes, copying from older functests (as I believe we all do)

Copy link
Member

Choose a reason for hiding this comment

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

A new one is both faster and better suited for the problem

Faster and better in the context of a given isolated problem solution task, but not so fast and good in the overall. Each new .test is another lilttle piece of stuff to mantain (and when you have to do a large refactor effort, e.g. XML removal, these little pieces, all together, have a significative cost). In addition, each new .test is a little more delay in make test, so impacting our development speed (even in the case of "I'm working on the next development while make test ends" part of your mind is still in the PR that you haven't merged yet due to the make test... at least in my case I cannot do complete context change as computers do ;).

It isnt' a golde rule about this, but something to think about (food for thought).

@@ -295,6 +297,7 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36})
=======================================
POST http://127.0.0.1:REGEX(\d+)/notify
Fiware-Servicepath: /
Fiware-Servicepath: /
Copy link
Member

Choose a reason for hiding this comment

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

This is an example of dup with an already existing "Fiware-Servicpath: /" line

@@ -320,6 +323,7 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36})
=======================================
POST http://127.0.0.1:REGEX(\d+)/notify
Fiware-Servicepath: /
Fiware-Servicepath: /
Copy link
Member

Choose a reason for hiding this comment

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

This is an example of dup with an already existing "Fiware-Servicpath: /" line

@fgalan
Copy link
Member

fgalan commented Jan 20, 2017

LGTM

@fgalan fgalan merged commit 54cea66 into master Jan 20, 2017
@fgalan fgalan deleted the bug/2584_service_path_as_http_header_in_notifications branch January 20, 2017 10:48
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