search

neoave / mrack

Multicloud use-case based multihost async provisioner for CIs and testing during development
Apache License 2.0
12 stars 14 forks source link

feat: Provide beaker log links #286

Closed skycastlelily closed 8 months ago

skycastlelily commented 9 months ago

Client like tmt may need to fetch the logs,store log contents for users

skycastlelily commented 9 months ago

Hi, Thanks for your feedback.

Should I assume that similar contribution will be made to tmt to use it?

Yes, and I'm gonna to write a unit test to cover this.

On Thu, Feb 1, 2024 at 4:16 PM Petr Vobornik @.***> wrote:

@.**** commented on this pull request.

Hi,

thanks for the patch.

Should I assume that similar contribution will be made to tmt to use it? In such case, I think it would make sense to also include a unit test which would test the thing tmt will expect. The reason is that mrack doesn't currently have any promise of internal API stability and thus some work can break this integration. Having a unit test might help highlight a potential breakage and thus either prevent it or start a conversation/change on tmt side.

In src/mrack/providers/beaker.py https://github.com/neoave/mrack/pull/286#discussion_r1473965367:

     resources = []

for recipe in eTree.fromstring(bkr_job_xml).iter("recipe"):

  • for logs in recipe.iter('logs'):
  • for log in logs.iter('log'):

There is an indentation error: https://dev.azure.com/neoave/mrack/_build/results?buildId=1135&view=logs&j=e4354d11-29aa-59c2-ce04-0d9aced3ea05&t=37a28340-749f-5255-d3d8-b83a7b4239ca&l=16

— Reply to this email directly, view it on GitHub https://github.com/neoave/mrack/pull/286#pullrequestreview-1855784214, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKFR23HVJNTGDDGUL4AMYOTYRNFNRAVCNFSM6AAAAABCUAMPW2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNJVG44DIMRRGQ . You are receiving this because you authored the thread.Message ID: @.***>

skycastlelily commented 9 months ago

Hi,

thanks for the patch.

Should I assume that similar contribution will be made to tmt to use it? In such case, I think it would make sense to also include a unit test which would test the thing tmt will expect. The reason is that mrack doesn't currently have any promise of internal API stability and thus some work can break this integration. Having a unit test might help highlight a potential breakage and thus either prevent it or start a conversation/change on tmt side.

Hi @pvoborni , I have scripted a unit test,which need the code from this https://github.com/neoave/mrack/pull/287,would you please review it ?Thanks:)

skycastlelily commented 8 months ago

Sorry for the late response, I'm just back from my vacation. I updated the code, the error should be fixed now:)

On Wed, Feb 14, 2024 at 11:43 PM Petr Vobornik @.***> wrote:

@.**** commented on this pull request.

The code looks good to me. But the test fails in CI: https://dev.azure.com/neoave/mrack/_build/results?buildId=1141&view=logs&jobId=e4354d11-29aa-59c2-ce04-0d9aced3ea05&j=e4354d11-29aa-59c2-ce04-0d9aced3ea05&t=c546a923-41d2-519e-602b-c74183bf4a4c

  • beaker lib tries to load /etc/beaker/client.conf

I amended the PR name to pass the check, but not sure why it failed again. Maybe it will be fixed in next push.

— Reply to this email directly, view it on GitHub https://github.com/neoave/mrack/pull/286#pullrequestreview-1880642342, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKFR23F62MWIWAXKYXUFFOLYTTLQLAVCNFSM6AAAAABCUAMPW2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQOBQGY2DEMZUGI . You are receiving this because you authored the thread.Message ID: @.***>