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

[WFLY-11261] CLI unable to refer jberet resources after run PurgeBatc… #11793

Merged
merged 1 commit into from Nov 1, 2018
Merged

[WFLY-11261] CLI unable to refer jberet resources after run PurgeBatc… #11793

merged 1 commit into from Nov 1, 2018

Conversation

nagetsum
Copy link
Contributor

…hlet

Please make sure your PR meets the following requirements:

  • Pull Request title is properly formatted: [WFLY-XYZ] Subject or WFLY-XYZ Subject
  • Pull Request contains link to the JIRA issue(s)
  • Pull Request contains description of the issue(s)
  • Pull Request does not include fixes for issues other than the main ticket
  • Attached commits represent units of work and are properly formatted

@wildfly-ci
Copy link

Can one of the admins verify this patch?

Copy link
Member

@jamezp jamezp left a comment

Choose a reason for hiding this comment

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

My only concern here is performance which we may need to look into. For a small number of jobs this shouldn't be an issue.

@bstansberry
Copy link
Contributor

@jamezp is hasJobExecution ok? On the surface it seems like it would be vulnerable to similar problems, since it just assumes the current children set is ok. So uses of it could provide a child Resource but then the runtime handling of that child resource might fail.

I suppose in the end in that kind of situation there's a good chance the request would fail any way, and this is just a matter of failing cleanly in Stage.MODEL with an understandable NoSuchResourceException vs failing in RUNTIME with a more obscure exception.

Anyway, as I wrote this I've convinced myself that any problem there is a different enough use case that there's no reason to mandate solving it as part of fixing this issue.

@jamezp
Copy link
Member

jamezp commented Oct 31, 2018

@bstansberry That's a good point and IIRC the reason it was done like that was for performance. I think it takes thousands, maybe tens of thousands, of jobs before performance is an issue though.

@jamezp
Copy link
Member

jamezp commented Oct 31, 2018

This is okay to test

@jamezp
Copy link
Member

jamezp commented Oct 31, 2018

I've filed a separate issue as a follow up for this https://issues.jboss.org/browse/WFLY-11264

@jamezp
Copy link
Member

jamezp commented Oct 31, 2018

this is okay to test

@jamezp
Copy link
Member

jamezp commented Oct 31, 2018

this is ok to test

@kabir kabir merged commit 5008a9c into wildfly:master Nov 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants