You seem to have included your virtualenv in the commit - could you remove that?

You also seem to have touched a lot of things that are best left alone. Also I think some issues have occurred with indenting/whitespace etc. Could you try and tidy that up?

The issue only needs some pretty minimal changes so could you try and tidy this up to only include the minimal adjustments necessary?

Yes, I will make changes and update soon.

There is something odd going on with the testing here. Not sure what as it seems unrelated to the code changes that you have made.

@saadkhi I think for the failure of the autopep8 test, when you created the pull request did you click the checkbox to allow upstream developers to make changes to your branch? My best guess at the moment for whats happening here is it's trying to create the autopep8 branch to pull against your branch Enh_int

@saadkhi Take a look at the GangaCore Integration tests. There are 3 errors there that relate to the code that you modified. Please take a look first yourself and then ask for help if you can't figure out what is wrong.

Check the full details. Extract below.

FAILED ganga/GangaCore/test/GPI/Monitoring/TestMonitoring.py::TestMonitoring::test_b_EnableMonitoring - TypeError: AsyncMonitoringService.runMonitoring() got an unexpected keyword argument 'steps'
FAILED ganga/GangaCore/test/GPI/Monitoring/TestMonitoring.py::TestMonitoring::test_g_runMonitoring_withJobIDList - TypeError: AsyncMonitoringService.runMonitoring() got an unexpected keyword argument 'steps'
FAILED ganga/GangaCore/test/GPI/Monitoring/TestMonitoring.py::TestMonitoring::test_h_runMonitoring_withJobObject - TypeError: AsyncMonitoringService.runMonitoring() got an unexpected keyword argument 'steps'

@saadkhi I think for the failure of the autopep8 test, when you created the pull request did you click the checkbox to allow upstream developers to make changes to your branch? My best guess at the moment for whats happening here is it's trying to create the autopep8 branch to pull against your branch Enh_int

I'm not sure about it, let me check and fix what is going wrong with it.

@saadkhi Take a look at the GangaCore Integration tests. There are 3 errors there that relate to the code that you modified. Please take a look first yourself and then ask for help if you can't figure out what is wrong.

Check the full details. Extract below.

FAILED ganga/GangaCore/test/GPI/Monitoring/TestMonitoring.py::TestMonitoring::test_b_EnableMonitoring - TypeError: AsyncMonitoringService.runMonitoring() got an unexpected keyword argument 'steps'
FAILED ganga/GangaCore/test/GPI/Monitoring/TestMonitoring.py::TestMonitoring::test_g_runMonitoring_withJobIDList - TypeError: AsyncMonitoringService.runMonitoring() got an unexpected keyword argument 'steps'
FAILED ganga/GangaCore/test/GPI/Monitoring/TestMonitoring.py::TestMonitoring::test_h_runMonitoring_withJobObject - TypeError: AsyncMonitoringService.runMonitoring() got an unexpected keyword argument 'steps'

yes, @egede I'm working on it to figure out why these test cases get failling.

@saadkhi I think for the failure of the autopep8 test, when you created the pull request did you click the checkbox to allow upstream developers to make changes to your branch? My best guess at the moment for whats happening here is it's trying to create the autopep8 branch to pull against your branch Enh_int

I’m having trouble understanding what autopep8 is and how it applies to my pull request. Could you please explain it, or if possible, arrange a meeting sometime this week to discuss?

I’m having trouble understanding what autopep8 is and how it applies to my pull request. Could you please explain it, or if possible, arrange a meeting sometime this week to discuss?

so PEP8 is the python style guidelines for coding. Autopep8 is a tool that when run over a codebase will convert all files to the PEP8 standard. As part of our CI/CD pipeline we run an action that automatically runs this autopep8 tool over the files that you have modified and creates a new branch/pull request with the changes necessary to meet the PEP8 standard.

Hopefully that is clear. If not, I can have a separate conversation with you or alternatively you could probably join our Monday morning (UK time) meeting.

I’m having trouble understanding what autopep8 is and how it applies to my pull request. Could you please explain it, or if possible, arrange a meeting sometime this week to discuss?

so PEP8 is the python style guidelines for coding. Autopep8 is a tool that when run over a codebase will convert all files to the PEP8 standard. As part of our CI/CD pipeline we run an action that automatically runs this autopep8 tool over the files that you have modified and creates a new branch/pull request with the changes necessary to meet the PEP8 standard.

Hopefully that is clear. If not, I can have a separate conversation with you or alternatively you could probably join our Monday morning (UK time) meeting.

Okay, I'll join the meeting and then will solve the autopep8 issue after meeting as soon as possible.

Okay, I'll join the meeting and then will solve the autopep8 issue after meeting as soon as possible.

👍🏻 The meeting is in 17 mins. Do you have the connection details? If you are on the mattermost channel there is a link at the top

So looking in the coverage of the testing I indeed see that the code that you have implemented is never executed. So in fact the entire runMonitoring function in Local_GangaMC_Service.py can be removed.

So the real question now is what the new test files are actually testing. At the moment they just look for runMonitoring returning True, but they should be extended to see if the jobs in question are actually monitored (and others not). It could be the argument is just ignored. Sorry @saadkhi but this is turning more complicated that we realised and we probably gave you the wrong advise to begin with.

So the real question now is what the new test files are actually testing. At the moment they just look for runMonitoring returning True, but they should be extended to see if the jobs in question are actually monitored (and others not). It could be the argument is just ignored. Sorry @saadkhi but this is turning more complicated that we realised and we probably gave you the wrong advise to begin with.

Hi @egede, thank you for your guidance. Initially, I created the test cases based on the Local_GangaMC_Service implementation in Ganga, but encountered errors during your testing process. After that, I noticed the runMonitoring functionality in the MonitoringService and adjusted the test cases accordingly. One more things, just tell that the issue is from my side thats why code is not merged.

I’ll continue to explore the Ganga project and improve my contributions alongside the team. I appreciate your support and feedback. Thank you again!

I think this went a bit off-piste, so closing.