enhancement/Automate Update the manifest refs to tags #4190
Conversation
Signed-off-by: Hitensh Kharva <[email protected]>
Signed-off-by: Hitensh Kharva <[email protected]>
Signed-off-by: Hitensh Kharva <[email protected]>
Signed-off-by: Hitensh Kharva <[email protected]>
hitenshKharva
requested review from
dblock,
peterzhuamazon,
bbarani,
gaiksaya,
rishabh6788,
zelinh,
jordarlu,
prudhvigodithi,
Divyaasm and
tianleh
as code owners
Signed-off-by: Hitensh Kharva <[email protected]>
Signed-off-by: Hitensh Kharva <[email protected]>
|
As discussed during office hours, @peterzhuamazon I have deleted the Github actions yml file. I will raise another PR for the yml file github actions. Thank you. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4190 +/- ##
==========================================
- Coverage 93.31% 93.29% -0.03%
==========================================
Files 187 187
Lines 5792 5802 +10
==========================================
+ Hits 5405 5413 +8
- Misses 387 389 +2 ☔ View full report in Codecov by Sentry. |
Thanks @hitenshKharva . I have converted this PR back to draft until you have all the test cases ready before review. Thanks! |
There was a problem hiding this comment.
Hi @hitenshKharva thanks for the initial draft,
Have a few comments and we can talk about the key implementations more either in officehour or in private.
This piece should first exist within src folder, either side in a existing workflow dir or in a new release_workflow dir instead of github workflow dir.
Next, the implementation is simply working as a piece of quick script and needs some structure compares to other workflows, it needs real arguments instead of grabbing user inputs from argv, for example.
Finally it needs a entrypoint to run these functions through classes instead of running as is. I would recommend reading through the build or assemble workflow to get an idea on how it is being shaped.
Happy to discuss more on the next step of improvement.
Thanks!
| # folder_path = f"/manifests/{new_version}" # Replace with the actual folder path | ||
|
|
||
| # Get the folder name and use it as the new version | ||
| new_version = os.path.basename(folder_path) |
There was a problem hiding this comment.
Instead of this, you should allow users to enter the manifest full path and retrieve the version from the actual yml file like in any other workflow in this repo.
There was a problem hiding this comment.
This was one of the possible workflow solutions in the issue discussions.
#2546 (comment)
| new_version = os.path.basename(folder_path) | ||
|
|
||
| # Define a list of component names that should have the same version as 'OpenSearch' and 'common-utils' | ||
| same_version_components = ['OpenSearch', 'common-utils'] |
There was a problem hiding this comment.
Only OpenSearch/OpenSearch-Dashboards/functionaltestrepo has same 3 digit version number, others will have 4 digit numbers with 0 as the 4th digit.
There was a problem hiding this comment.
I will check this out and update the code accordingly.
| # Iterate through YAML files in the folder and update the 'ref' in each file | ||
| for filename in os.listdir(folder_path): | ||
| if filename.endswith('.yml'): | ||
| yaml_file = os.path.join(folder_path, filename) | ||
| with open(yaml_file, 'r') as file: | ||
| data = yaml.safe_load(file) | ||
| for component in data.get('components', []): | ||
| if component['name'] in same_version_components: | ||
| ref = component.get('ref', 'Not found') | ||
| component['ref'] = f'tags/{new_version}' | ||
| else: | ||
| component['ref'] = f'tags/{new_version}.0' | ||
|
|
||
| with open(yaml_file, 'w') as file: | ||
| yaml.dump(data, file) |
There was a problem hiding this comment.
We only care about the input manifest with this and we dont care about the test manifest and others for now.
You could consider only working on the yml user pointed to you instead of the folder approach as there might be outliers or make the scope too complicated.
| with open(yaml_file, 'w') as file: | ||
| yaml.dump(data, file) | ||
|
|
||
| print(f"Ref values updated in the specified YAML files in the folder. New version: {new_version}") |
There was a problem hiding this comment.
You should consider using the logging instead of using plain printf.
|
Closing this PR in favor of #4868 |
Description
Describe what this change achieves.
Issues Resolved
Issue
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.