Disable trigger of cos_integration in PR

[rgildein]

The cos_integration workflow should be triggered only by dispatch or, when the workflow was changed and not for all PR.

fixes: #344

[Deezzir]

LGTM

[samuelallan72]

From #344 :

The cos_integration.yaml is triggered during PR, however the integration test are not building charm locally.

Why not build the charm locally then so we can run the cos_integration tests on PRs? I don't understand the rationale for dropping the tests - it's important to run all CI tests on PRs to catch issues before they are merged.

Also, why does this not work currently when run on PRs, but does work on manual dispatch or weekly tests?

[rgildein]

Why not build the charm locally then so we can run the cos_integration tests on PRs? I don't understand the rationale for dropping the tests - it's important to run all CI tests on PRs to catch issues before they are merged.

I think that this tests was designed to run only weekly, @jneo8 ? I'm ok to use builded charm for testing this with latest/stable channel. However, running it with latest/edge in PR is not good aproach, since it can easialy failed and halt the PR, so that's why I think tunning on latest/edge should by only done by weekly test.

Also, why does this not work currently when run on PRs, but does work on manual dispatch or weekly tests?

I think that there is some issue with new release of cos in latested/edge, which is blocking my other PR.

[aieri]

This integration test isn't testing the code in a pending PR, so it doesn't add value as a gate to merging. @rgildein is creating a separate issue to track adapting the test code to make it more appropriate for testing PRs

overriding to land this PR and unblock others - we can rediscuss this change internally though!

[samuelallan72]

This integration test isn't testing the code in a pending PR, so it doesn't add value as a gate to merging

Ah right, so it's only testing hardware-observer-operator from the charm store? That makes more sense for why it's a weekly test then.

[jneo8]

Note: The purpose of this test is to make sure the newest cos integration can work with hardware observer. Weekly is fine but the purpose is to remind someone to check with cos team to make sure we are on the path.

[rgildein]

Note: The purpose of this test is to make sure the newest cos integration can work with hardware observer. Weekly is fine but the purpose is to remind someone to check with cos team to make sure we are on the path.

Yes, that purpose makes sense to me. What do you say about running it with PR and latest/stable and on weekly test we run it with latest/edge? Check this #349 issue.

BTW, Test are failing for almost a week, we need to investigate why.

[samuelallan72]

Note: The purpose of this test is to make sure the newest cos integration can work with hardware observer. Weekly is fine but the purpose is to remind someone to check with cos team to make sure we are on the path.

Could we add this purpose, and the fact that it tests hardware observer from the charm store (not from the repo), as a comment at least in the cos_integration.yaml file please? Otherwise we're bound to have this misunderstanding again.

[rgildein]

There is an issue #349 for running these test in every PR, so for now comment is not needed. But I agree that's confusing.