-
Notifications
You must be signed in to change notification settings - Fork 332
Add check of component versions to CI #414
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
Changes from all commits
139837d
e3df59b
b6f6167
bf10dc5
1d9cda7
b8d4b04
7a9ff50
9acea33
5d3e0cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -141,7 +141,7 @@ Run from `bundle-workflow` before making pull requests. | |
| cd bundle-workflow | ||
|
|
||
| pipenv run isort . | ||
| pipenv run black . | ||
| git status -s | grep -e "[MA?]\s.*.py" | cut -c4- | xargs pipenv run black | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is cryptic...
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trust me. It just picks all .py files that are added or modified, minus the deleted ones. It's a dev readme anyway, so you're supposed to parse regexp in your head ;)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I opened #442 to add a pre-commit hook. |
||
| pipenv run flake8 | ||
| pipenv run pytest | ||
| pipenv run mypy . | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # | ||
| # The OpenSearch Contributors require contributions made to | ||
| # this file be licensed under the Apache-2.0 license or a | ||
| # compatible open source license. | ||
|
|
||
| from abc import ABC, abstractmethod | ||
|
|
||
|
|
||
| class Check(ABC): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Nit] This class seems like it is doing double duty, its got a description of a git repo, version and build target information as well as an interface for validation something about the check itself. Could resolve by making it a VersionCheck, or BuildCheck, or breaking them data fields into another object.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. I'll refactor this separately since it's the same pattern in |
||
| def __init__(self, component, git_repo, version, arch, snapshot): | ||
| self.component = component | ||
| self.git_repo = git_repo | ||
| self.version = version | ||
| self.arch = arch | ||
| self.snapshot = snapshot | ||
| self.opensearch_version = version + "-SNAPSHOT" if snapshot else version | ||
| self.component_version = version + ".0-SNAPSHOT" if snapshot else f"{version}.0" | ||
| if self.component.name == "OpenSearch": | ||
| # HACK: OpenSearch version is 3-digits | ||
| self.component_version = self.opensearch_version | ||
|
|
||
| @abstractmethod | ||
| def check(self): | ||
| pass | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # | ||
| # The OpenSearch Contributors require contributions made to | ||
| # this file be licensed under the Apache-2.0 license or a | ||
| # compatible open source license. | ||
|
|
||
| import logging | ||
| import re | ||
|
|
||
| from ci_workflow.check import Check | ||
| from system.properties_file import PropertiesFile | ||
|
|
||
|
|
||
| class CheckGradleDependencies(Check): | ||
| def __init__( | ||
| self, component, git_repo, version, arch, snapshot, gradle_project=None | ||
| ): | ||
| super().__init__(component, git_repo, version, arch, snapshot) | ||
| self.gradle_project = gradle_project | ||
| self.dependencies = self.__get_dependencies() | ||
|
|
||
| def __get_dependencies(self): | ||
| cmd = " ".join( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Optional] Seems like there is shared constructions between
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose if we do more gradle commands for sure. |
||
| [ | ||
| f"./gradlew {self.gradle_project or ''}:dependencies", | ||
| f"-Dopensearch.version={self.opensearch_version}", | ||
| f"-Dbuild.snapshot={str(self.snapshot).lower()}", | ||
| '| grep -e "---"', | ||
|
Comment on lines
+25
to
+28
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just trying to understand: Also for next steps, we should verify plugin dependencies on other plugins and make sure their versions also match the manifest.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you know how to get just those? I can open a separate PR for improving this.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah a simple way is to do something like Eg: From our fav plugin AD |
||
| ] | ||
| ) | ||
|
|
||
| lines = self.git_repo.output(cmd) | ||
| stack = ["root"] | ||
| props = PropertiesFile("") | ||
| for line in lines.split("\n"): | ||
| # e.g. "| +--- org.opensearch:opensearch-core:1.1.0-SNAPSHOT" | ||
| # see job_scheduler_dependencies.txt in tests for an example | ||
| match = re.search(r"---\s(.*):([0-9,\w,.-]*)[\s]*", line) | ||
|
dblock marked this conversation as resolved.
|
||
| if match: | ||
| levels = line.count(" ") + line.count("---") | ||
|
|
||
| while levels < len(stack): | ||
| del stack[-1] | ||
|
|
||
| if levels == len(stack): | ||
| stack[-1] = match.group(1).strip() | ||
| elif levels > len(stack): | ||
| stack.append(match.group(1).strip()) | ||
|
|
||
| key = "/".join(stack) | ||
| value = match.group(2).strip() | ||
| logging.debug(f"{key}={value}") | ||
| props[key] = value | ||
|
|
||
| return props | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are not backporting java 11 to 1.0 branch, can we start with 11 here and make a separate job for the 1.0 manifests?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything is still on 14.