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

Add export capability and version property #72

Merged
merged 11 commits into from
Nov 1, 2024

Conversation

Gijsreyn
Copy link
Contributor

@Gijsreyn Gijsreyn commented Oct 22, 2024


Addresses issue #71

@Gijsreyn
Copy link
Contributor Author

Hi @ryfu-msft, I know I'm bugging you, but if you have some time to review this one, that would be cool.

I'm preparing the pipelines also to start supporting MacOS/Linux agents. That would be interesting if we can add a matrix in the Azure Pipelines (#64). I have tested now in a demo project using macos-latest and ubuntu-latest to test with dsc.exe Unfortunately, I created the following issue as I'm scratching my head why it doesn't work.

@ryfu-msft
Copy link
Contributor

/azp run

Copy link

Azure Pipelines failed to run 1 pipeline(s).

utilities/scripts/Set-EnvironmentOnAgent.ps1 Outdated Show resolved Hide resolved
pipelines/azure-pipelines.yml Outdated Show resolved Hide resolved
resources/PythonPip3Dsc/PythonPip3Dsc.psm1 Show resolved Hide resolved
resources/PythonPip3Dsc/PythonPip3Dsc.psm1 Outdated Show resolved Hide resolved
resources/PythonPip3Dsc/PythonPip3Dsc.psm1 Outdated Show resolved Hide resolved
resources/PythonPip3Dsc/PythonPip3Dsc.psm1 Outdated Show resolved Hide resolved
resources/PythonPip3Dsc/PythonPip3Dsc.psm1 Show resolved Hide resolved
resources/PythonPip3Dsc/PythonPip3Dsc.psm1 Show resolved Hide resolved
resources/PythonPip3Dsc/PythonPip3Dsc.psm1 Outdated Show resolved Hide resolved
resources/PythonPip3Dsc/PythonPip3Dsc.psm1 Outdated Show resolved Hide resolved
@ryfu-msft
Copy link
Contributor

ryfu-msft commented Oct 28, 2024

Hi @ryfu-msft, I know I'm bugging you, but if you have some time to review this one, that would be cool.

I'm preparing the pipelines also to start supporting MacOS/Linux agents. That would be interesting if we can add a matrix in the Azure Pipelines (#64). I have tested now in a demo project using macos-latest and ubuntu-latest to test with dsc.exe Unfortunately, I created the following issue as I'm scratching my head why it doesn't work.

Apologies for the delay on this review, definitely keep tagging me if you need me to take another look quickly. Let's address ubuntu and macosx support in a separate PR. I will need to look into setting up the proper matrix so that it'll be easier for you to build on that.

@Gijsreyn
Copy link
Contributor Author

Hey champ, thanks for the review. I have worked on your remarks and solved nearly all of them. All test should pass and I have included one update test:

image

I cannot see why the pipeline is failing. Is it possible to open up the project for public viewing? Would really appreciate it.

Regarding your remark on the matrix and other OS support. Let me explain what I'm trying to do, then I'll leave it up to decide whether we want to include the setup script.

My feeling steers towards individual releases of modules. The way how pipelines are now setup, doesn't support it, nor does it have a publishing stage (I can create a separate issue for that if wanted). To be more efficient, it would make sense to have separate pipelines for each module being published. Adding the script, reduces duplicate code and also:

  • Allows for generic installation across templatized Azure Pipelines
  • Each module should have it's own bootstrapping fo the required software in the Pester tests (or separate scripts in the utilities folder and called in the Pester test).

I know it is not related to this PR, but wanted to add the script borrowed from the AVM team. I slightly modified it to work with PSResourceGet. Most agents have PowerShell 7.2+, so it doesn't require bootstrapping for PowerShellGet. That's why I switched between Install-Module to Install-PSResource. This copes for MacOs/Ubuntu.

If you have information regarding the OSes I can target, I can give a shot at building the matrix. Cheers Ryan.

@ryfu-msft
Copy link
Contributor

/azp run

Copy link

Azure Pipelines failed to run 1 pipeline(s).

@ryfu-msft
Copy link
Contributor

Hey champ, thanks for the review. I have worked on your remarks and solved nearly all of them. All test should pass and I have included one update test:

image

I cannot see why the pipeline is failing. Is it possible to open up the project for public viewing? Would really appreciate it.

Regarding your remark on the matrix and other OS support. Let me explain what I'm trying to do, then I'll leave it up to decide whether we want to include the setup script.

My feeling steers towards individual releases of modules. The way how pipelines are now setup, doesn't support it, nor does it have a publishing stage (I can create a separate issue for that if wanted). To be more efficient, it would make sense to have separate pipelines for each module being published. Adding the script, reduces duplicate code and also:

  • Allows for generic installation across templatized Azure Pipelines
  • Each module should have it's own bootstrapping for the required software in the Pester tests (or separate scripts in the utilities folder and called in the Pester test).

I know it is not related to this PR, but wanted to add the script borrowed from the AVM team. I slightly modified it to work with PSResourceGet. Most agents have PowerShell 7.2+, so it doesn't require bootstrapping for PowerShellGet. That's why I switched between Install-Module to Install-PSResource. This copes for MacOs/Ubuntu.

If you have information regarding the OSes I can target, I can give a shot at building the matrix. Cheers Ryan.

I unfortunately can't open up the pipeline for security reasons. I did look at your pipeline and the issue is that your alignment isn't correct in your yaml. Everything under steps: should be in line with - job. Also the path to your script isn't correct. It should be Utilities\Scripts\Set-EnvironmentOnAgent.ps1.

Regarding the script, I still don't know how I feel about incorporating this into our pipeline. I don't see a need to have separate bootstrapping steps for each module as currently each test is responsible for its own setup as you've done with python. We have separate internal publishing pipelines for each module. The main purpose of this pipeline is just to run tests to make sure nothing breaks. I can see some benefits for handling installation of macos or ubuntu, but I would imagine we would define different steps for each OS matrix. Let's move this to a separate PR as I would like to check in your work and have this PR be scoped to just improving python dsc.

@Gijsreyn
Copy link
Contributor Author

Gijsreyn commented Oct 31, 2024

I understand Ryan. Thanks for the discussion nevertheless. @ryfu-msft Just reverted the pipeline and fixed the tests. Can you please run it again for me?

@ryfu-msft
Copy link
Contributor

Only owners of this repo can do that, sorry 😞

@ryfu-msft
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Gijsreyn
Copy link
Contributor Author

Sorry for the spam Ryan, it just crossed my mind. Are you open for a discussion regarding the topic for cross-platform pipeline? :)

@Gijsreyn
Copy link
Contributor Author

image

Just pushed other changes for another try.

@ryfu-msft
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ryfu-msft
Copy link
Contributor

Sorry for the spam Ryan, it just crossed my mind. Are you open for a discussion regarding the topic for cross-platform pipeline? :)

Of course, I started a discussion thread for this topic that way it doesn't get buried in this PR:

#86

@Gijsreyn
Copy link
Contributor Author

Gijsreyn commented Nov 1, 2024

@ryfu-msft I think what is going wrong. I tinkered the .psm1 file and the test to first install Python before discovering Pip. Can you trigger it once more?

@ryfu-msft
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ryfu-msft ryfu-msft merged commit 9abaa5b into microsoft:main Nov 1, 2024
4 checks passed
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.

2 participants