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

Remove package retrieval when verify pkg version #7585

Merged
merged 2 commits into from
Jan 31, 2024

Conversation

raych1
Copy link
Member

@raych1 raych1 commented Jan 25, 2024

The script fails to find the package for the Go language because the Go language doesn't generate a package for release; instead, it simply pushes the code to the GitHub repository.

However, since the Verify-PackageVersion function doesn't actually use the package found by the APIView related function, I removed that part of the code. Now, the script works for these five languages by retrieving package information from the {packagename}.json file.

In addition, I added the validation for Go data plane scenario.

Test run result:
Go
JS

@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

Make sure it is present in eng/scripts/Language-Settings.ps1 and referenced in eng/common/scripts/common.ps1.`
See https://github.com/Azure/azure-sdk-tools/blob/main/doc/common/common_engsys.md#code-structure"
exit 1
}
if (-not $PackageInfoDirectory) {
Copy link
Member

Choose a reason for hiding this comment

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

Before this was only for local testing but now after your change this will happen every time and I suspect that isn't what we want. The idea was that we would get the already existing packageInfo from the artifacts but it appears that go doesn't publish those. So, the question is should we have go start publishing them? We might need to for other scenarios like APIView. However if we don't do that we shouldn't write the json files to disk we should just call the helper functions that the at save script is calling.

See https://github.com/Azure/azure-sdk-tools/blob/main/eng/common/scripts/Save-Package-Properties.ps1#L95

For example there will be cases where looking for $PackageName.json will fail that were already handled in that save script https://github.com/Azure/azure-sdk-tools/blob/main/eng/common/scripts/Save-Package-Properties.ps1#L115

@weshaggard
Copy link
Member

@praveenkuttappan @benbp this means that the APIView requirement check doesn't work for go. Is that a know gap? Did we never implement that block for go?

@benbp
Copy link
Member

benbp commented Jan 25, 2024

@praveenkuttappan Praveen Kuttappan FTE @benbp Ben Broderick Phillips FTE this means that the APIView requirement check doesn't work for go. Is that a know gap? Did we never implement that block for go?

@weshaggard to my knowledge this block was working, we implemented it last year.

@weshaggard
Copy link
Member

@praveenkuttappan Praveen Kuttappan FTE @benbp Ben Broderick Phillips FTE this means that the APIView requirement check doesn't work for go. Is that a know gap? Did we never implement that block for go?

@weshaggard to my knowledge this block was working, we implemented it last year.

OK looks like go doesn't use the PackageInfo files https://github.com/Azure/azure-sdk-for-go/blob/main/eng/scripts/Language-Settings.ps1#L137 it is looking for apireview files directly which is why this doesn't work in our general case. We might want to have go start producing those PackageInfo files as artifacts so things can be more consistent and we have that info for other parts of the pipeline.

@raych1
Copy link
Member Author

raych1 commented Jan 25, 2024

@praveenkuttappan Praveen Kuttappan FTE @benbp Ben Broderick Phillips FTE this means that the APIView requirement check doesn't work for go. Is that a know gap? Did we never implement that block for go?

@weshaggard to my knowledge this block was working, we implemented it last year.

@benbp @weshaggard API view works in Go. The point is Go SDK doesn't have package generated like other language which is by design. We could publish the API view artifacts which is *.gosource then we can use it in this script but this artifact is not real Go SDK package. I don't recommend this approach although it will work.
Alternatively, we could inject the script in the pipeline yml like here or we publish the PackageInfo after it is created then it can be used in this script. This is only need for Go scenario as for other four languages, PackageInfo has been already published as artifacts.

@raych1
Copy link
Member Author

raych1 commented Jan 26, 2024

@weshaggard , I added the step to publish the PackageInfo in Go pipeline. Please see the linked PR for details.

@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

azure-sdk added a commit to Azure/azure-sdk-for-js that referenced this pull request Jan 31, 2024
@raych1 raych1 merged commit 37c14df into main Jan 31, 2024
11 checks passed
@raych1 raych1 deleted the user/raych1/remove-package-retrieval branch January 31, 2024 08:33
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.

4 participants