-
Notifications
You must be signed in to change notification settings - Fork 18
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
Refactor McM REST client. #23
Comments
Hi @tvami, @efeyazgan, @lmoureaux, @DickyChant, @sihyunjeon I am working on a refactor for the McM REST client to give more flexibility on the components it uses for performing authenticated HTTP requests to CERN web services mainly focusing on:
I would like to know your opinion about how smooth would it be to integrate this new version into some of the projects/scripts you work with, like:
This update only requires handling isolated I. Do you think it would be easy to set up the runtime environment ( II. Is there something else you think should be added in this new version to make users’ lives simpler by interacting with the application to operate a big set of requests programmatically (without directly using the UI)? III. Do you know other projects that heavily rely on this package? If so, and their source is public, could you include a URL to them below? Thanks, |
Hi @ggonzr I will try to test this tomorrow but will the access be the same, e.g. mcm._McM__get('public/restapi/requests/get/%s' % (prepid)) ? Thanks, |
This is really great news! Meanwhile performing test as Efe did, I think I can quickly share some pieces of experience from my end in response to your questions.
And for your questions
|
Yes it is currently compatible but we may update in the future the code to remove mangled methods (like __get(...)) and just rename it with an underscore (_get(...)) $ git clone --depth 1 https://github.com/cms-sw/genproductions.git
Cloning into 'genproductions'...
remote: Enumerating objects: 23625, done.
.......
$ python3 -V && python3 -m venv venv && source ./venv/bin/activate && pip install git+https://github.com/cms-PdmV/mcm_scripts.git@refactor/rest-client
Python 3.9.18
Collecting git+https://github.com/cms-PdmV/mcm_scripts.git@refactor/rest-client
......
Resolved https://github.com/cms-PdmV/mcm_scripts.git to commit e92c1ef2aacc9a7a5d3d63b731c64e6a5b282917
Installing build dependencies ... done
......
(venv)$ cd ./genproductions/bin/utils/
.....
(venv)$ python3 test_request_fragment_check.py
[test_request_fragment_check][2024-06-21 10:31:23,897][INFO] Using Python version: 3.9.18 (main, Jan 24 2024, 00:00:00)
[GCC 11.4.1 20231218 (Red Hat 11.4.1-3)
......
[2024-06-21 10:31:24 +0200][WARNING][http_client.mcm]: Using McM client without providing authentication
Running on McM DEV!
......
***********************************************************************************
Number of warnings = 1
WARNINGS:
----------
1 : No parton shower weights configuration in the fragment. Since the Fall18 campaign, we recommend to include Parton Shower weights
----------
Number of errors = 0
.
----------------------------------------------------------------------
Ran 1 test in 1.814s
OK |
Thanks! I had checked with some other bunch of requests as well. It seems all OK, so I assume no changes needed in the request checking script itself. |
And yes it would be nice to rename the "mangled methods". Thanks. |
Thanks for the feedback, related to your questions:
Not at this time, modifications to the web application to expose more endpoints as public are outside the scope of this update. Ideally, in the medium to long term, we would also like to remove any public endpoints that the McM application currently has, even if only used for read-only purposes. Instead, if it is only needed to perform read-only operations accessible to the import os
from json import dumps
from pathlib import Path
import requests
from rest import McM
from rest.client.session import SessionFactory
def set_up_client_credentials() -> requests.Session:
client_id = os.getenv("ACCESS_TOKEN_CLIENT_ID", "")
client_secret = os.getenv("ACCESS_TOKEN_CLIENT_SECRET", "")
url = "https://cms-pdmv-dev.web.cern.ch/mcm/"
credential_path = Path.home() / Path("test-credential")
target_application = "cms-ppd-pdmv-dev"
return SessionFactory.configure_by_access_token(
url=url,
credential_path=credential_path,
client_id=client_id,
client_secret=client_secret,
target_application=target_application,
)
# Create an instance without setting an authentication method
# Then replace the session.
mcm = McM(id=None)
mcm.session = set_up_client_credentials()
# Consume resources using the private API
# Example took from: https://github.com/cms-PdmV/mcm_scripts/blob/master/get_requests.py
requests_query = """
B2G-Fall13-00001
B2G-Fall13-00005 -> B2G-Fall13-00015
"""
range_of_requests = mcm.get_range_of_requests(requests_query)
print('Found %s requests' % (len(range_of_requests)))
for request in range_of_requests:
print(request['prepid']) For registering a client application, it is required to follow the instructions available here. This example allows users' code to access all private endpoints and query the required information. If this proposal fulfills the requirement, please let us know so that we can update the McM client to offer this kind of authentication too.
Yes, it would require encapsulating the GEN script execution into an EL9 container in a similar way # Execute some internals with newer Python versions.
cat <<'EndOfTestFile' > PPD-TestForGENScriptDoNotOperateThis-00001_pre.sh
#!/bin/bash
# Python interpreter version to use.
PYTHON_VERSION='python3.9'
# Install PdmV HTTP Package
if ! eval "${PYTHON_VERSION} -V"; then
echo "Python version: ${PYTHON_VERSION} is not available..."
exit 1
fi
# Install the package and an isolated venv
PYTHON_VENV_NAME="isolated-venv-${PYTHON_VERSION}"
${PYTHON_VERSION} -m venv ${PYTHON_VENV_NAME}
source "./${PYTHON_VENV_NAME}/bin/activate"
pip install git+https://github.com/cms-PdmV/mcm_scripts.git@refactor/rest-client
echo "Environment packages...\n"
echo $($PYTHON_VERSION -V)
pip freeze
echo 'Starting execution...\n\n'
# GEN Script begin
rm -f request_fragment_check.py
wget -q https://raw.githubusercontent.com/cms-sw/genproductions/master/bin/utils/request_fragment_check.py
# Remove all instructions that import remote modules
sed -i '/sys.path.append(/d' request_fragment_check.py
chmod +x request_fragment_check.py
./request_fragment_check.py --bypass_status --prepid PPD-TestForGENScriptDoNotOperateThis-00001 --dev --develop
GEN_ERR=$?
if [ $GEN_ERR -ne 0 ]; then
echo "GEN Checking Script returned exit code $GEN_ERR which means there are $GEN_ERR errors"
echo "Validation WILL NOT RUN"
echo "Please correct errors in the request and run validation again"
exit $GEN_ERR
fi
echo "Running VALIDATION. GEN Request Checking Script returned no errors"
# GEN Script end
# Download fragment from McM
curl -s -k https://cms-pdmv-dev.web.cern.ch/mcm/public/restapi/requests/get_fragment/PPD-TestForGENScriptDoNotOperateThis-00001 --retry 3 --create-dirs -o Configuration/GenProduction/python/PPD-TestForGENScriptDoNotOperateThis-00001-fragment.py
[ -s Configuration/GenProduction/python/PPD-TestForGENScriptDoNotOperateThis-00001-fragment.py ] || exit $?;
# Check if fragment contais gridpack path and that it is in cvmfs
if grep -q "gridpacks" Configuration/GenProduction/python/PPD-TestForGENScriptDoNotOperateThis-00001-fragment.py; then
if ! grep -q "/cvmfs/cms.cern.ch/phys_generator/gridpacks" Configuration/GenProduction/python/PPD-TestForGENScriptDoNotOperateThis-00001-fragment.py; then
echo "Gridpack inside fragment is not in cvmfs."
exit -1
fi
fi
# End of PPD-TestForGENScriptDoNotOperateThis-00001_pre.sh file
EndOfTestFile
chmod +x PPD-TestForGENScriptDoNotOperateThis-00001_pre.sh
singularity run -B /afs -B /eos -B /cvmfs -B /etc/grid-security -B /etc/pki/ca-trust --home $PWD:$PWD /cvmfs/unpacked.cern.ch/registry.hub.docker.com/cmssw/el9:x84_64 $(echo "$(pwd)/PPD-TestForGENScriptDoNotOperateThis-00001_pre.sh")
# `cms-sw` execution works as normally expected
....... Related to your lastest two items, sure I will forward this issue to more colleagues for asking their opinion about this refactor. |
Updates to include this version for the McM automatic scripts will be deferred and added as part of cms-PdmV/cmsPdmV#1124 instead. The last is because these automatic scripts also use sections of the web application code that are currently being migrated from Python 2 to Python 3. The new version of the McM REST client is only compatible with Python 3. Therefore, apart from this, all other tasks are completed. |
Perform several updates to enhance the code quality for this module and its re-usability in more contexts than requests for McM. Split the responsibility related to authenticating an HTTP client for performing requests from the per-application operations to consume a required resource. Also, allow extensions for other endpoints related to other PdmV applications and improve the performance handling requests.
Is your feature related to a problem?
Because of the McM REST client module tying HTTP methods to application-specific operations, users who require a fast way to authenticate other kinds of requests end up consuming mangled methods to achieve this behavior. This is not expected as it creates a problem in performing internal updates on modules without breaking existing client implementations.
Also, several modules in PdmV's integrations use this implementation for McM and change some internals to perform requests to other applications, bypassing the original purpose of the module and creating issues to maintain it in the future. To conclude, the current implementation doesn’t allow to handle requests concurrently and this creates performance issues related to bulk big processing loads.
Describe the solution you'd like
requests
library as the HTTP client instead of default implementations with core modules to improve flexibility, maintenance, and performance.request.Session
) in another class. Include hooks to retrieve or request tokens and session cookies to authenticate operations and refresh the credentials when required. Store ID/OAuth tokens and refresh them if possible to improve UX and avoid extra human interventions.McM scripts
.Expected behavior
Users have an HTTP client they can use to perform authenticated HTTP requests to CERN internal applications and, related to our context, perform operations to PdmV applications in an easier and high-performance way.
The text was updated successfully, but these errors were encountered: