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

Fixes #5580: check the monotony of promises timestamp #531

Conversation

ncharles
Copy link
Member

@ncharles ncharles commented Oct 7, 2014

No description provided.

"${g.rudder_tools_updated_origin}"
create => "true",
edit_defaults => empty,
edit_line => insert_lines("${sys.date}"),
edit_line => insert_lines("${sys.systime}"),
Copy link
Member

Choose a reason for hiding this comment

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

doesn't this append a new line each time ? ending with a full history of timestamps ?

Copy link
Member Author

Choose a reason for hiding this comment

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

no, edit_defaults empty empties the file, so it can contain only one line

@peckpeck
Copy link
Member

It's all good for me

@ncharles
Copy link
Member Author

@peckpeck if it's all good, can you merge it ?

@jooooooon
Copy link
Member

Let me review this first.

@peckpeck
Copy link
Member

peckpeck commented Apr 3, 2015

@ncharles This PR is not mergeable anymore
@jooooooon is your review positive ?

@jooooooon
Copy link
Member

On 3 April 2015 10:35:49 CEST, peckpeck notifications@github.com wrote:

@ncharles This PR is not mergeable anymore
@jooooooon is your review positive ?


Reply to this email directly or view it on GitHub:
#531 (comment)

If I recall correctly, we decided not to merge this change to reduce risk, and keep it for a better time. Maybe 3.1 is a good time for this?

I don't think I have reviewed it, so don't worry.

Sent from Kaiten Mail. Please excuse my brevity.

@peckpeck
Copy link
Member

@ncharles any update ?

@jooooooon
Copy link
Member

For what it's worth, I have now reviewed this code and it looks good to me. I just have one question: How does the rudder_tools_updated_origin file get created initially? The code here won't create it if it's missing, this is why I ask (but it could easily be changed to do so - maybe it should by default so that there is no risk of it not existing?)

I propose to merge this into master (for 3.2 / 4.0 version) soon, so that we have extensive testing during the next few months before it hits a release.

@ncharles
Copy link
Member Author

The file is created in distributePolicy/1.0/propagatePromises.cf , when tools are copied on the rudder root server

@ncharles ncharles closed this Aug 23, 2015
@ncharles
Copy link
Member Author

replaced by #735

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants