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

Fix CI #335

Merged
merged 4 commits into from
Aug 30, 2022
Merged

Fix CI #335

merged 4 commits into from
Aug 30, 2022

Conversation

Tompage1994
Copy link
Collaborator

What does this PR do?

Fixes broken CI where a non existant EE was referenced

How should this be tested?

CI should pass

Is there a relevant Issue open for this?

N/A

Other Relevant info, PRs, etc.

#334 introduced a CI failure

@Tompage1994
Copy link
Collaborator Author

Confirmed this does fix this issue but there is another issue with the diff section

@Tompage1994
Copy link
Collaborator Author

Think I've found the fix. No idea why the number has changed. Not sure I fully understand why it is a number (I guess ID)

Copy link
Collaborator

@djdanielsson djdanielsson left a comment

Choose a reason for hiding this comment

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

Is there a reason we use a number instead of a name?

@Tompage1994
Copy link
Collaborator Author

ok, this still doesn't work. The number seems to be different each time which means this isn't going to work. I assume this is all because the controller_api lookup returns the inventory id rather than the name. not sure what we can really do about that in this case. @sean-m-sullivan any ideas?

Copy link
Collaborator

@djdanielsson djdanielsson left a comment

Choose a reason for hiding this comment

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

you should be able to use name instead of number... I think worst case we take out the diff tests and wait until it gets rewritten to add it back in

@sean-m-sullivan
Copy link
Collaborator

The problem is in the test itself, its getting back numbers from the API, and in the future we could look it up, or something like that, its probably best just to disable the test till we can fix it.

Commented out IS diff test as broken and not easily fixable
@Tompage1994
Copy link
Collaborator Author

Ok, I've done that. We shall see how much further it gets now

Remove option deprecated from awx.awx
@Tompage1994
Copy link
Collaborator Author

It failed much later this time because a field has been deprecated and removed.

See ansible/awx#12366

@Tompage1994 Tompage1994 changed the title Update templates.yml Fix CI Aug 30, 2022
Copy link
Collaborator

@sean-m-sullivan sean-m-sullivan left a comment

Choose a reason for hiding this comment

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

lGTM

Copy link
Collaborator

@djdanielsson djdanielsson left a comment

Choose a reason for hiding this comment

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

seems to be working now, sweet!

@sean-m-sullivan sean-m-sullivan merged commit 01c04c3 into devel Aug 30, 2022
@Tompage1994 Tompage1994 deleted the Tompage1994-patch-2 branch September 1, 2022 14:35
przemkalit pushed a commit to przemkalit/aap_configuration that referenced this pull request Nov 22, 2024
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