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.

Sign up for GitHub

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

refactor: implementation for consuming internal or external artifactory #442

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

refactor: implementation for consuming internal or external artifactory #442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
475e462
func:changes done for consuming internal or external artifactory
shwetaupadhya Feb 22, 2024
92d4e75
code style issue fixed
shwetaupadhya Feb 22, 2024
0522155
Merge branch 'main' of https://github.com/ansys/ansys-templates into …
shwetaupadhya Feb 23, 2024
fab3c22
Update ansys-saf-desktop version to 1.0rc4
ravi-krsharma Feb 23, 2024
19c9d1c
Merge branch 'suadhyay/func/consume-external-artifactory' of https://…
ravi-krsharma Feb 23, 2024
7187065
Update poetry lock and pyproject toml as per ansys-saf-desktop
ravi-krsharma Feb 26, 2024
590ea40
Update ansys-saf-desktop to 1.1.0
ravi-krsharma Feb 28, 2024
11c5755
Update lock file as per saf desktop
ravi-krsharma Feb 28, 2024
adf49b3
Merge branch 'main' into suadhyay/func/consume-external-artifactory
ravi-krsharma Feb 29, 2024
fd12930
Update pyproject.toml and poetry.lock
ravi-krsharma Mar 4, 2024
0438e07
Updated lock file
ravi-krsharma Mar 4, 2024
36922b8
Updated saf desktop verson
ravi-krsharma Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 22 additions & 23 deletions src/ansys/templates/python/solution/{{cookiecutter.__project_name_slug}}/poetry.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions src/ansys/templates/python/solution/{{cookiecutter.__project_name_slug}}/pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ priority = "supplemental"

[tool.poetry.dependencies]
python = ">=3.8.1, <3.11"
ansys-saf-portal = {version = "1.0rc3", source = "solutions-private-pypi"}
ansys-saf-desktop = {version = "1.0.0", source = "solutions-private-pypi"}
ansys-saf-portal = {version = "1.0rc3"}
ansys-saf-desktop = {version = "1.1.0"}
{% if cookiecutter.with_dash_ui == "yes" %}
ansys-web-components-dash = {version = "^0.0.dev2", source = "solutions-private-pypi"}
ansys-solutions-dash-lib = {version = "^0.4", source = "solutions-private-pypi"}
ansys-web-components-dash = {version = "^0.0.dev2"}
ansys-solutions-dash-lib = {version = "^0.4"}
dash = "^2.6"
dash_bootstrap_components = "^1.2"
dash-extensions = ">=0.1.13"
Expand Down Expand Up @@ -129,4 +129,4 @@ select = ['D107']
source = ["ansys.solutions"]

[tool.coverage.report]
show_missing = true
show_missing = true
112 changes: 94 additions & 18 deletions ...ansys/templates/python/solution/{{cookiecutter.__project_name_slug}}/setup_environment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ def install_build_system(args: object) -> None:
build_system_executable = DEPENDENCY_MANAGER_PATHS[sys.platform]["build_sys_exec"]
if os.path.exists(build_system_executable):
if sys.platform == "win32":
subprocess.run(
process = subprocess.run(
[
"powershell",
"-Command",
Expand All @@ -591,16 +591,22 @@ def install_build_system(args: object) -> None:
f"'{build_system_executable}'",
]
)
if process.returncode == 0:
check_if_user_is_internal_or_external()
elif sys.platform == "linux":
subprocess.run(
process = subprocess.run(
[
"ln",
"-sf",
build_system_executable,
DEPENDENCY_MANAGER_PATHS[sys.platform]["dep_bin_venv_path"],
]
)
if process.returncode == 0:
check_if_user_is_internal_or_external()
return
else:
check_if_user_is_internal_or_external()
print("Skipped")
print()

Expand Down Expand Up @@ -655,31 +661,41 @@ def configure_poetry(
if not use_private_sources:
return

is_token_exist = False
# Declare credentials for private sources
for source in private_sources:
print(f"Declare credentials for {source['name']}")
USERNAME = "PAT"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
USERNAME = "PAT"
username = "PAT"

token = None
if source["name"].lower() == "pypi":
continue
elif source["url"] == "https://pkgs.dev.azure.com/pyansys/_packaging/pyansys/pypi/simple/":
token = os.environ["PYANSYS_PYPI_PRIVATE_PAT"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
token = os.environ["PYANSYS_PYPI_PRIVATE_PAT"]
token = os.environ.get("PYANSYS_PYPI_PRIVATE_PAT")

elif source["url"] == "https://pkgs.dev.azure.com/pyansys/_packaging/ansys-solutions/pypi/simple/":
token = os.environ["PYANSYS_PYPI_PRIVATE_PAT"]
else:
raise Exception(f"Unknown private source {source['name']} with url {source['url']}.")
token = os.environ.get("PYANSYS_PYPI_PRIVATE_PAT")
elif source["url"] == "https://artifactory.ansys.com/artifactory/api/pypi/saf_pypi/simple/":
token = os.environ.get("ARTIFACTORY_PRIVATE_PYPI_PAT")
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be consistent with the ENV vars - notice the order of the pyansys and solutions vars are PYPI_PRIVATE and the new one is PRIVATE_PYPI

USERNAME = os.environ.get("ARTIFACTORY_PRIVATE_PYPI_USER")

is_token_exist = bool(token) if not is_token_exist else is_token_exist

# Store credentials
if credentials_management_method == "keyring":
# Declare source URL
command_line = [poetry_executable, "config", f"repositories.{source['name']}", source["url"], "--local"]
subprocess.run(command_line, check=True)
# Declare source credentials
command_line = [poetry_executable, "config", f"http-basic.{source['name']}", "PAT", token, "--local"]
subprocess.run(command_line, check=True)
elif credentials_management_method == "environment-variables":
# Format source name to comply with Poetry environment variable syntax
source_name = source["name"].upper().replace("-", "_")
# Create Poetry environment variable
os.environ[f"POETRY_HTTP_BASIC_{source_name}_USERNAME"] = "PAT"
os.environ[f"POETRY_HTTP_BASIC_{source_name}_PASSWORD"] = token
if token and USERNAME:
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably have an else that displays an error

if credentials_management_method == "keyring":
# Declare source URL
command_line = [poetry_executable, "config", f"repositories.{source['name']}", source["url"], "--local"]
subprocess.run(command_line, check=True)
# Declare source credentials
command_line = [poetry_executable, "config", f"http-basic.{source['name']}", USERNAME, token, "--local"]
subprocess.run(command_line, check=True)
elif credentials_management_method == "environment-variables":
# Format source name to comply with Poetry environment variable syntax
source_name = source["name"].upper().replace("-", "_")
# Create Poetry environment variable
os.environ[f"POETRY_HTTP_BASIC_{source_name}_USERNAME"] = USERNAME
os.environ[f"POETRY_HTTP_BASIC_{source_name}_PASSWORD"] = token
if not is_token_exist:
raise Exception("Please provide valid token for private sources")


def check_dependency_group(dependency_group: str, configuration: str) -> bool:
Expand Down Expand Up @@ -932,6 +948,66 @@ def install_dotnet_linux_dependencies():
shell=True,
)

def check_if_user_is_internal_or_external():
private_pypi_pats = [
os.environ.get("PYANSYS_PRIVATE_PYPI_PAT"),
Copy link
Contributor

Choose a reason for hiding this comment

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

These values are duplicated a few times in this file, so we should probably pull them out as variables

os.environ.get("SOLUTIONS_PRIVATE_PYPI_PAT")
]
is_internal = any(private_pypi_pats)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
is_internal = any(private_pypi_pats)
internal_source_configured = any(private_pypi_pats)

private_sources_dict = {
'solutions-private-pypi': {'url': 'https://pkgs.dev.azure.com/pyansys/_packaging/ansys-solutions/pypi/simple/', 'priority': 'supplemental'},
'pyansys-private-pypi': {'url': 'https://pkgs.dev.azure.com/pyansys/_packaging/pyansys/pypi/simple/', 'priority': 'supplemental'},
'artifactory-private-pypi': {'url': 'https://artifactory.ansys.com/artifactory/api/pypi/saf_pypi/simple/', 'priority': 'supplemental'}
}
private_sources = get_private_sources(DEPENDENCY_MANAGER_PATHS["common"]["configuration_file"])
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private_sources = get_private_sources(DEPENDENCY_MANAGER_PATHS["common"]["configuration_file"])
toml_private_sources = get_private_sources(DEPENDENCY_MANAGER_PATHS["common"]["configuration_file"])

private_sources = [source for source in private_sources if source['name'] != 'PyPI']
internal_sources = {'solutions-private-pypi', 'pyansys-private-pypi'}
Copy link
Contributor

Choose a reason for hiding this comment

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

The internal vs. private naming is very confusing to me - shouldn't they all be private?

Also, the external part is equally confusing because you still need to be authenticated. For naming, we should either stick to the source (internal/external) or the access (private/public)

external_sources = {'artifactory-private-pypi'}
all_sources = {source['name'] for source in private_sources}

# Add sources if they are not already present
sources_to_add = set()
if is_internal:
sources_to_add |= internal_sources - all_sources
else:
sources_to_add |= external_sources - all_sources

commands = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally, I don't like this at all. This means that the TOML file isn't really deterministic - it depends on whatever ENV vars are set when you run the setup script. I think this will result in a lot of unnecessary confusion as developers commit versions of the TOML file switching back and forth between solutions/pyansys vs. artifactory.

I don't know what the requirements are for the artifactory access, so it is difficult for me to propose an alternative solution. I know portal uses 2 TOML files, which I also don't like.

My only other suggestion is to dynamically switch from solutions/pyansys to artifactory during the build/package/release process in CI. However, this would need to be configurable because there may be situations where we want to build a package for internal consumption that depends on prerelease package versions that aren't available in artifactory.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking out loud - another option is to test how poetry handles private repositories that aren't authenticated. If this is handled gracefully, we could simply add all the potential sources as supplemental sources and let poetry figure out which ones are authenticated properly.

[
DEPENDENCY_MANAGER_PATHS[sys.platform]["build_sys_exec"],
"source",
"add",
source,
private_sources_dict[source]['url'],
f"--priority={private_sources_dict[source]['priority']}"
] for source in sources_to_add
]

# Remove sources that should not be present
sources_to_remove = set()
if is_internal:
sources_to_remove |= all_sources - internal_sources
else:
sources_to_remove |= all_sources - external_sources

commands += [
[
DEPENDENCY_MANAGER_PATHS[sys.platform]["build_sys_exec"],
"source",
"remove",
source
] for source in sources_to_remove
]

for command in commands:
process = subprocess.run(
command,
check=True,
shell=DEPENDENCY_MANAGER_PATHS[sys.platform]["shell"]
)
if process.returncode != 0:
break


def main() -> None:
"""Sequence of operations leading to the complete Python ecosystem."""
Expand Down
Loading