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

AnsibleRunner: reset lastEvent #545

Closed
wants to merge 1 commit into from
Closed

Conversation

mnecas
Copy link
Member

@mnecas mnecas commented Jul 21, 2022

When the runner gets LastEventId: 0 and totalEvents: 0. The client does not find any new events so we have null event and break the while(true).

String jobEvents = getJobEventsDir(playUuid);
while(true){
String event = getNextEvent(playUuid, lastEventId);
if (event == null) {
break;

The AnsibleCallback is using a singleton of the AnsibleRunnerClient because of this we get the previous run lastEvent which is not empty so we call getLastEventId`.

In the getLastEventId we get the return of the prefix index of the file which is the lastEventId and because it's from the previous run it overrides the current id.

public int getLastEventId() {
return Integer.valueOf(lastEvent.split("-")[0]);
}

Copy link
Member

@michalskrivanek michalskrivanek left a comment

Choose a reason for hiding this comment

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

but this is the onl break out of the infinite loop. So the return value condition is ineffective AFAICT and can just return lastEventId always?

Copy link
Member

@michalskrivanek michalskrivanek left a comment

Choose a reason for hiding this comment

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

It might be best - or even required, looking at playHasEnded - to eliminate lastEvent completely

@mnecas
Copy link
Member Author

mnecas commented Jul 21, 2022

Could be... this is more question for @dangel101... in what situation we need lastEvent?

@michalskrivanek
Copy link
Member

Use the grep, Luke!

Could be... this is more question for @dangel101... in what situation we need lastEvent?

http://bezdelnique.ru/wp-content/uploads/2009/08/use_source_luke-500x388.png

@mnecas
Copy link
Member Author

mnecas commented Jul 21, 2022

Use the grep, Luke!

Could be... this is more question for @dangel101... in what situation we need lastEvent?

http://bezdelnique.ru/wp-content/uploads/2009/08/use_source_luke-500x388.png

what I understand from code is it seems like we don't need it at all and we could just use lastEventId

@mnecas
Copy link
Member Author

mnecas commented Jul 21, 2022

/ost

1 similar comment
@mnecas
Copy link
Member Author

mnecas commented Jul 21, 2022

/ost

@michalskrivanek
Copy link
Member

I've meant something more like #546

@mnecas mnecas mentioned this pull request Jul 22, 2022
@michalskrivanek
Copy link
Member

replaced by #546 and #547

@dangel101
Copy link
Member

Could be... this is more question for @dangel101... in what situation we need lastEvent?

While we don't know how many events we expect, we do know that in any case the last event contains the run's stats. So that helps to determine if a run has actually ended

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.

3 participants