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

Enhancement - Leverage requests session object for network level retry management #183

Merged
merged 43 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
621fb65
test small instance runner
joshburt Jan 9, 2024
fc2443a
alternate
joshburt Jan 11, 2024
8259152
stashing working
joshburt Jan 11, 2024
9d78866
cleanup logging
joshburt Jan 11, 2024
4521d51
re-enable the tests
joshburt Jan 12, 2024
b55a87d
stashing
joshburt Jan 19, 2024
7b3869d
Merge branch 'master' of github.com:Anaconda-Platform/ae5-tools into …
joshburt Jan 19, 2024
9597678
consolidate cli tests
joshburt Jan 23, 2024
b2c078b
updateds test for recent changes to master
joshburt Jan 25, 2024
b07864a
system test fixes
joshburt Jan 26, 2024
e716af1
revert
joshburt Jan 26, 2024
99b5e81
pipeline and doc updates
joshburt Jan 26, 2024
38c5d60
set back to original less noisy approach (easier for troubleshooting)
joshburt Jan 26, 2024
962e7d1
fixes for cli repsonse marshalling changes
joshburt Jan 26, 2024
3e7aae5
cleanup of code
joshburt Jan 26, 2024
03bb54b
typo
joshburt Jan 26, 2024
168fb28
do not persist credentials in tests
joshburt Jan 30, 2024
3be6811
setup for troubleshooting
joshburt Feb 5, 2024
142f3fa
Trigger CI
pww217 Feb 6, 2024
60c49db
Update pull-request.yml
pww217 Feb 6, 2024
dfc6c92
migrated system and load tests to different repo
joshburt Feb 6, 2024
5accc74
Merge branch 'jb_retries' of github.com:Anaconda-Platform/ae5-tools i…
joshburt Feb 6, 2024
9c7b6a6
linting fix
joshburt Feb 6, 2024
25f0bfa
debug
joshburt Feb 6, 2024
bbc83b7
logging
joshburt Feb 6, 2024
68c1768
debug creation
joshburt Feb 6, 2024
98c5832
gha package creation debugging
joshburt Feb 6, 2024
73718d0
explicitly add conda-build
joshburt Feb 6, 2024
1afaab1
Merge branch 'master' of github.com:Anaconda-Platform/ae5-tools into …
joshburt Feb 6, 2024
5ff7d62
revert
joshburt Feb 6, 2024
66ed75e
externalize the build
joshburt Feb 7, 2024
531e830
add cloudflare headers support
joshburt Feb 8, 2024
4e433ea
ensure its set up on creation
joshburt Feb 8, 2024
19855b6
logging
joshburt Feb 8, 2024
63915f0
logging adjustments
joshburt Feb 8, 2024
eab5731
load cf headers on auth
joshburt Feb 8, 2024
f8570ba
logging
joshburt Feb 9, 2024
e0816aa
cleanup
joshburt Feb 9, 2024
245431a
cleanup exception handling
joshburt Feb 9, 2024
6b86d06
remove debugging
joshburt Feb 9, 2024
943464d
test coverage additions
joshburt May 10, 2024
e6cd793
more context
joshburt May 10, 2024
1f6a1c2
linting change
joshburt May 10, 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
11 changes: 1 addition & 10 deletions .github/workflows/pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ on:
jobs:
build:
runs-on: ubuntu-latest
# runs-on: self-hosted
# runs-on: self-hosted-amd64-small-privileged
# runs-on: self-hosted-amd64-large-privileged-on-demand-storage
defaults:
run:
shell: bash -el {0}
Expand Down Expand Up @@ -40,16 +37,10 @@ jobs:
- name: Build Conda Package
run: |
mkdir build
conda build conda-recipe --output-folder build
./build-package.sh
- name: Run Integration Tests
run: |
anaconda-project run test:integration:slipstream
# - name: Run System Tests
# env:
# AE5_HOSTNAME: dev1.ae.anacondaconnect.com
# CI: true
# run: |
# anaconda-project run test:system
- name: Upload to anaconda.org (Dev Build)
env:
ANACONDA_TOKEN: ${{ secrets.ANACONDA_TOKEN }}
Expand Down
5 changes: 1 addition & 4 deletions .github/workflows/python-publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ on:
jobs:
package:
runs-on: ubuntu-latest
# runs-on: self-hosted
# runs-on: self-hosted-amd64-small-privileged
# runs-on: self-hosted-amd64-large-privileged-on-demand-storage
defaults:
run:
shell: bash -el {0}
Expand All @@ -33,7 +30,7 @@ jobs:
- name: Build Conda Package
run: |
mkdir build
conda build conda-recipe --output-folder build
./build-package.sh
- name: Upload to anaconda.org
env:
ANACONDA_TOKEN: ${{ secrets.ANACONDA_TOKEN }}
Expand Down
1 change: 0 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ These commands are used during development for solution management.
| test | Default | Run all test suites |
| test:unit | Default | Unit Test Suite |
| test:integration | Default | Integration Test Suite |
| test:system | Default | System Test Suite |

## Contributing

Expand Down
2 changes: 1 addition & 1 deletion LICENSE
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Copyright 2023 Anaconda, Inc.
Copyright 2024 Anaconda, Inc.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ There is already a fair amount of inline help, so type `ae5 --help` to get start
For k8s service integration refer to:
* [k8s Service Component Documentation](docs/source/K8S_SERVER.md)
* [Project Commands Documentation](docs/source/COMMANDS.md)
* [System and Load Testing](docs/source/SYSTEM_TESTS.md)
* [Contributing](CONTRIBUTING.md)

## Installation
Expand Down Expand Up @@ -66,9 +65,10 @@ The package has the following particular dependencies:
- `revision`: `list`, `info`, `download`
- `sample`: `info`, `list`
- `secret`: `list`, `add`, `delete`
- `role`: `add`, `remove`
- `run`: `delete`, `info`, `list`, `log`, `stop`
- `session`: `branches`, `changes`, `info`, `list`, `open`, `start`, `stop`
- `user`: `info`, `list`
- `user`: `info`, `list`, `create`, `delete`
- Simple commands: `call`, `login`, `logout`
- Login options: `--hostname`, `--username`, `--admin-username`, `--admin-hostname`, `--impersonate`
- Output format options: `--format`, `--filter`, `--columns`, `--sort`, `--width`, `--wide`, `--no-header`
Expand Down
107 changes: 97 additions & 10 deletions ae5_tools/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,13 @@

import requests
from dateutil import parser
from requests import Session
from requests.adapters import HTTPAdapter
from requests.packages import urllib3
from urllib3 import Retry

from .archiver import create_tar_archive
from .common.config.environment import demand_env_var, get_env_var
from .config import config
from .docker import build_image, get_condarc, get_dockerfile
from .filter import filter_list_of_dicts, filter_vars, split_filter
Expand Down Expand Up @@ -222,8 +226,6 @@ def __init__(self, response, method, url, **kwargs):
msg.append(f' json: {kwargs["json"]}')
super(AEUnexpectedResponseError, self).__init__("\n".join(msg))

pass


class AESessionBase(object):
"""Base class for AE5 API interactions."""
Expand All @@ -250,15 +252,60 @@ def __init__(self, hostname, username, password, prefix, persist):
self.password = password
self.persist = persist
self.prefix = prefix.lstrip("/")
self.session = requests.Session()
self.session.verify = False
self.session.cookies = LWPCookieJar()
self.session: Session = AESessionBase._build_requests_session()

# Cloudflare headers need to be present on all requests (even before auth can be start).
self._set_cf_headers()

# Proceed with auth flow
if self.persist:
self._load()
self.connected = self._connected()
if self.connected:
self._set_header()

def _set_cf_headers(self):
"""
If cloudflare auth is enabled, then get and set the headers
https://developers.cloudflare.com/cloudflare-one/identity/service-tokens/#connect-your-service-to-access
CF-Access-Client-Id: <Client ID>
CF-Access-Client-Secret: <Client Secret>
"""

if get_env_var(name="CF_ACCESS_CLIENT_ID") and get_env_var(name="CF_ACCESS_CLIENT_SECRET"):
self.session.headers["CF-Access-Client-Id"] = demand_env_var(name="CF_ACCESS_CLIENT_ID")
self.session.headers["CF-Access-Client-Secret"] = demand_env_var(name="CF_ACCESS_CLIENT_SECRET")

@staticmethod
def _build_requests_session() -> Session:
"""
Responsible for creating the requests session object.
This implementation is global right now, but future work should allow more granular
control of retries on a per-call basis.
"""

session: Session = Session()
session.cookies = LWPCookieJar()

# TODO: This should be parameterized
session.verify = False

# Status Code Defaults
# 403, 501, 502 are seen when ae5 is behind CloudFlare
# 502, 503, 504 can be encountered when ae5 is under heavy load
# TODO: this should be definable on a per command basis, and parameterized.
retries: Retry = Retry(
total=10,
backoff_factor=0.1,
status_forcelist=[403, 502, 503, 504],
allowed_methods={"POST", "PUT", "PATCH", "GET", "DELETE", "OPTIONS", "HEAD"},
)

adapter: HTTPAdapter = HTTPAdapter(max_retries=retries)
session.mount(prefix="https://", adapter=adapter)

return session

@staticmethod
def _auth_message(msg, nl=True):
print(msg, file=sys.stderr, end="\n" if nl else "")
Expand Down Expand Up @@ -291,6 +338,9 @@ def _is_login(self, response):
pass

def authorize(self):
# Cloudflare headers need to be present on all requests (even before auth can be start).
self._set_cf_headers()

key = f"{self.username}@{self.hostname}"
need_password = self.password is None
last_valid = True
Expand Down Expand Up @@ -594,6 +644,9 @@ def _set_header(self):
s.headers["x-xsrftoken"] = cookie.value
break

# Ensure that Cloudflare headers get added [back] to session when setting the other auth headers.
self._set_cf_headers()

def _load(self):
s = self.session
if os.path.exists(self._filename):
Expand Down Expand Up @@ -1787,11 +1840,45 @@ def _set_header(self):
self.session.headers["Authorization"] = f'Bearer {self._sdata["access_token"]}'

def _connect(self, password):
resp = self.session.post(
self._login_base + "/token",
data={"username": self.username, "password": password, "grant_type": "password", "client_id": "admin-cli"},
)
self._sdata = {} if resp.status_code == 401 else resp.json()
try:
# Set the initial security data to an empty dictionary.
self._sdata = {}

# Get our auth
params: dict = {"username": self.username, "password": password, "grant_type": "password", "client_id": "admin-cli"}
resp: requests.Response = self.session.post(
self._login_base + "/token",
data=params,
)

if resp.status_code not in [401]:
try:
self._sdata = resp.json()
except json.decoder.JSONDecodeError as error:
# The response is not json parsable.
# This is most likely some type of error (serialized, or html content, etc).
print(f"Received an unexpected response.\nStatus Code: {resp.status_code}\n{resp.text}")

except requests.exceptions.RetryError:
message: str = f"Exceeded maximum retry limit on call to {self._login_base}/token"
try:
message += f", response code seen: {resp.status_code}, last response: {resp.text}"
except NameError:
# if `resp` is not defined, then we hit the retry max before it was declared
# during the `session.post` operation.
pass

print(message)

except Exception as error:
message: str = f"Unknown error calling {self._login_base}/token"
try:
message += f", response code seen: {resp.status_code}, last response: {resp.text}"
except NameError:
# if `resp` is not defined, just pass.
pass
print(message)
print(str(error))

def _disconnect(self):
if self._sdata:
Expand Down
1 change: 1 addition & 0 deletions ae5_tools/common/config/environment.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
""" Helper functions for environment variables. """

from __future__ import annotations

import os
Expand Down
10 changes: 0 additions & 10 deletions anaconda-project.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,6 @@ commands:
conda install build/noarch/ae5-tools-*.tar.bz2
py.test --cov=ae5_tools -v tests/integration --cov-append --cov-report=xml -vv

test:load:
env_spec: default
unix: |
python -m tests.load.runner

test:system:
env_spec: default
unix: |
python -m tests.system.runner

# Documentation Commands ####################################################

build:apidocs:
Expand Down
14 changes: 14 additions & 0 deletions build-package.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/bin/bash

# Perform the build
conda build conda-recipe --no-anaconda-upload --no-test --output-folder build

# Check the exit status of the cp command
if [ $? -eq 0 ]; then
echo "Build successful"
else
echo "Build failed"
fi

echo "Moving on ..."
exit 0
3 changes: 0 additions & 3 deletions docs/source/COMMANDS.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ These commands are used during project development.
| test | Default | Run all test suites |
| test:unit | Default | Unit Test Suite |
| test:integration | Default | Integration Test Suite |
| test:system | Default | System Test Suite |
| test:load | Default | Load Test Suite |


### Runtime

Expand Down
3 changes: 2 additions & 1 deletion docs/source/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,10 @@ The package has the following particular dependencies:
- `revision`: `list`, `info`, `download`
- `sample`: `info`, `list`
- `secret`: `list`, `add`, `delete`
- `role`: `add`, `remove`
- `run`: `delete`, `info`, `list`, `log`, `stop`
- `session`: `branches`, `changes`, `info`, `list`, `open`, `start`, `stop`
- `user`: `info`, `list`
- `user`: `info`, `list`, `create`, `delete`
- Simple commands: `call`, `login`, `logout`
- Login options: `--hostname`, `--username`, `--admin-username`, `--admin-hostname`, `--impersonate`
- Output format options: `--format`, `--filter`, `--columns`, `--sort`, `--width`, `--wide`, `--no-header`
Expand Down
Loading
Loading