Skip to content

Add E2E test for simple auth example#1148

Merged
rapids-bot[bot] merged 19 commits intoNVIDIA:developfrom
dagardner-nv:david-simple-auth-e2e
Nov 5, 2025
Merged

Add E2E test for simple auth example#1148
rapids-bot[bot] merged 19 commits intoNVIDIA:developfrom
dagardner-nv:david-simple-auth-e2e

Conversation

@dagardner-nv
Copy link
Contributor

@dagardner-nv dagardner-nv commented Nov 4, 2025

Description

  • Add the ability to optionally pass session keyword arguments with run_workflow
  • Add Oauth example service to tests/test_data/docker-compose.services.yml
  • Change Beautiful Soup dependency to use the beautifulsoup4 name and not the deprecated bs4 package name.

By Submitting this PR I confirm:

  • I am familiar with the Contributing Guidelines.
  • We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
    • Any contribution which contains commits that are not Signed-Off will not be accepted.
  • When the PR is ready for review, new or existing tests cover these changes.
  • When the PR is ready for review, the documentation is up to date with these changes.

Summary by CodeRabbit

Release Notes

  • Tests

    • Introduced OAuth2 Authorization Code flow testing infrastructure with new fixtures for server connectivity and client credential management.
  • New Features

    • Added session configuration support to workflow execution through new optional parameters.
  • Chores

    • Updated HTML parsing dependencies to current versions across packages.

…to the test services

Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
…the beautifulsoup4 name and not the deprecated bs4 name

Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
… the real handler would

Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
…into david-simple-auth-e2e

Signed-off-by: David Gardner <dagardner@nvidia.com>
@dagardner-nv dagardner-nv self-assigned this Nov 4, 2025
@dagardner-nv dagardner-nv added improvement Improvement to existing functionality non-breaking Non-breaking change labels Nov 4, 2025
@coderabbitai
Copy link

coderabbitai bot commented Nov 4, 2025

Walkthrough

The PR integrates OAuth2 server infrastructure into CI/CD and testing environments, updates HTML parsing dependencies across example projects, and extends the workflow execution API to support custom session parameters for authentication scenarios.

Changes

Cohort / File(s) Summary
OAuth2 CI/CD Configuration
.gitlab-ci.yml, tests/test_data/docker-compose.services.yml
Adds OAuth2 CI variables (NAT_CI_OAUTH2_HOST, NAT_CI_OAUTH2_PORT) and configures a new OAuth2 server service in GitLab CI test stage and Docker Compose stack with environment variables and health checks.
Dependency Updates (BeautifulSoup4)
pyproject.toml, examples/documentation_guides/workflows/text_file_ingest/pyproject.toml, examples/evaluation_and_profiling/email_phishing_analyzer/pyproject.toml, examples/frameworks/multi_frameworks/pyproject.toml
Replaces deprecated bs4==0.0.2 with beautifulsoup4~=4.13 across project dependencies for HTML parsing.
OAuth2 Testing Fixtures
packages/nvidia_nat_test/src/nat/test/plugin.py
Introduces oauth2_server_url_fixture and oauth2_client_credentials_fixture pytest fixtures to establish OAuth2 server connectivity and retrieve client credentials via HTTP requests and HTML parsing.
OAuth2 Integration Test
examples/front_ends/simple_auth/tests/test_simple_auth.py
Adds test_full_workflow integration test with _auth_callback helper to exercise OAuth2 Authorization Code flow through the authentication workflow.
Workflow API Extensions
src/nat/utils/__init__.py, packages/nvidia_nat_test/src/nat/test/utils.py
Updates run_workflow signatures to accept session_kwargs and **kwargs, enabling custom session parameters and forwarding of additional options through the workflow execution stack.

Sequence Diagram(s)

sequenceDiagram
    actor Test
    participant workflow as Workflow Engine
    participant session as SessionManager
    participant oauth2 as OAuth2 Server
    participant api as Target API

    Test->>+workflow: run_workflow(config, session_kwargs)
    workflow->>workflow: Load config & build Workflow
    workflow->>+session: Create SessionManager
    session->>+oauth2: Open session with session_kwargs
    oauth2-->>-session: Session established
    workflow->>+api: Perform workflow operations
    api-->>-workflow: Results
    workflow->>session: Execute within session context
    session-->>-workflow: Session result
    workflow-->>-Test: Return typed result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • API signature changes in src/nat/utils/__init__.py and test utils require verification that session context management is correctly implemented and backward compatible
  • OAuth2 fixture implementation in packages/nvidia_nat_test/src/nat/test/plugin.py involves external HTTP calls and HTML parsing logic that should be validated for robustness
  • Integration test logic in examples/front_ends/simple_auth/tests/test_simple_auth.py orchestrates multiple authentication steps and requires careful review of credential handling
  • Homogeneous dependency updates across multiple pyproject.toml files are straightforward but should verify no version conflicts

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding an end-to-end test for the simple auth example, which is the primary objective reflected in the PR.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
…into david-simple-auth-e2e

Signed-off-by: David Gardner <dagardner@nvidia.com>
@dagardner-nv dagardner-nv marked this pull request as ready for review November 4, 2025 19:50
@dagardner-nv dagardner-nv requested review from a team as code owners November 4, 2025 19:50
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/nat/utils/__init__.py (1)

28-33: Document the new session_kwargs parameter in the docstring.

The function signature now includes a session_kwargs parameter, but the docstring's Parameters section (lines 38-47) doesn't document it. Users need to understand what session arguments are supported.

Add documentation for the new parameter:

     to_type : type[_T]
         The type to convert the result to. Default is str.
+    session_kwargs : dict[str, typing.Any] | None
+        Optional keyword arguments to pass to the SessionManager.session() method. Supports parameters like
+        user_authentication_callback, http_connection, user_input_callback, etc. Default is None.
 
     Returns
packages/nvidia_nat_test/src/nat/test/utils.py (1)

71-89: Update docstring to document new parameters.

The function signature now includes assert_expected_answer and **kwargs parameters, but the docstring doesn't document them. Per coding guidelines, all parameters should be documented in Google-style docstrings.

As per coding guidelines.

     """
     Test specific wrapper for `nat.utils.run_workflow` to run a workflow with a question and validate the expected
     answer. This variant always sets the result type to `str`.
+
+    Parameters
+    ----------
+    config : Config | None
+        The configuration object to use for the workflow. If None, config_file must be provided.
+    config_file : StrPath | None
+        The path to the configuration file. If None, config must be provided.
+    question : str
+        The question/prompt to run the workflow with.
+    expected_answer : str
+        The expected answer to validate against the workflow result.
+    assert_expected_answer : bool
+        Whether to assert that the expected answer is present in the result. Default is True.
+    **kwargs
+        Additional keyword arguments forwarded to `nat.utils.run_workflow` (e.g., session_kwargs).
+
+    Returns
+    -------
+    str
+        The result of the workflow as a string.
     """
🧹 Nitpick comments (2)
packages/nvidia_nat_test/src/nat/test/plugin.py (1)

654-724: Consider improving exception handling and documenting the return value structure.

The fixture implementation works but has a few areas for improvement:

  1. Docstring: The return value structure should be documented, especially the dict keys: id, secret, username, url, cookies.

  2. Exception handling: The broad except Exception catches all errors. While acceptable for test fixtures that skip on failure, static analysis flags this pattern (ruff BLE001, B904).

Improvement 1: Document return value:

 @pytest.fixture(name="oauth2_client_credentials", scope="session")
 def oauth2_client_credentials_fixture(oauth2_server_url: str, fail_missing: bool) -> dict[str, typing.Any]:
     """
     Fixture to provide OAuth2 client credentials for testing
 
     Simulates the steps a user would take in a web browser to create a new OAuth2 client as documented in:
     examples/front_ends/simple_auth/README.md
+
+    Returns
+    -------
+    dict[str, typing.Any]
+        A dictionary containing:
+        - id: The OAuth2 client ID
+        - secret: The OAuth2 client secret
+        - username: The username used to create the client
+        - url: The OAuth2 server base URL
+        - cookies: Session cookies for authenticated requests
     """

Improvement 2: Be more specific with exception handling:

     except Exception as e:
         reason = f"Unable to create OAuth2 client: {e}"
         if fail_missing:
-            raise RuntimeError(reason)
+            raise RuntimeError(reason) from e
         pytest.skip(reason=reason)
examples/front_ends/simple_auth/tests/test_simple_auth.py (1)

59-59: Use descriptive variable name instead of ___.

The triple underscore ___ is unclear. Use a descriptive name like state_param or simply _ if the value is intentionally unused.

-    auth_url, ___ = oauth_client.create_authorization_url(url=config.authorization_url, state=state)
+    auth_url, _ = oauth_client.create_authorization_url(url=config.authorization_url, state=state)
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ad3cc7b and 937353d.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (10)
  • .gitlab-ci.yml (2 hunks)
  • examples/documentation_guides/workflows/text_file_ingest/pyproject.toml (1 hunks)
  • examples/evaluation_and_profiling/email_phishing_analyzer/pyproject.toml (1 hunks)
  • examples/frameworks/multi_frameworks/pyproject.toml (1 hunks)
  • examples/front_ends/simple_auth/tests/test_simple_auth.py (1 hunks)
  • packages/nvidia_nat_test/src/nat/test/plugin.py (1 hunks)
  • packages/nvidia_nat_test/src/nat/test/utils.py (1 hunks)
  • pyproject.toml (1 hunks)
  • src/nat/utils/__init__.py (2 hunks)
  • tests/test_data/docker-compose.services.yml (1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)

Files:

  • pyproject.toml
  • examples/documentation_guides/workflows/text_file_ingest/pyproject.toml
  • examples/evaluation_and_profiling/email_phishing_analyzer/pyproject.toml
  • packages/nvidia_nat_test/src/nat/test/plugin.py
  • tests/test_data/docker-compose.services.yml
  • src/nat/utils/__init__.py
  • examples/frameworks/multi_frameworks/pyproject.toml
  • packages/nvidia_nat_test/src/nat/test/utils.py
  • examples/front_ends/simple_auth/tests/test_simple_auth.py
{**/pyproject.toml,uv.lock}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

New dependencies must be added to both pyproject.toml (alphabetically) and uv.lock via ‘uv pip install --sync’

Files:

  • pyproject.toml
  • examples/documentation_guides/workflows/text_file_ingest/pyproject.toml
  • examples/evaluation_and_profiling/email_phishing_analyzer/pyproject.toml
  • examples/frameworks/multi_frameworks/pyproject.toml
**/*

⚙️ CodeRabbit configuration file

**/*: # Code Review Instructions

  • Ensure the code follows best practices and coding standards. - For Python code, follow
    PEP 20 and
    PEP 8 for style guidelines.
  • Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
    Example:
    def my_function(param1: int, param2: str) -> bool:
        pass
  • For Python exception handling, ensure proper stack trace preservation:
    • When re-raising exceptions: use bare raise statements to maintain the original stack trace,
      and use logger.error() (not logger.exception()) to avoid duplicate stack trace output.
    • When catching and logging exceptions without re-raising: always use logger.exception()
      to capture the full stack trace information.

Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any

words listed in the ci/vale/styles/config/vocabularies/nat/reject.txt file, words that might appear to be
spelling mistakes but are listed in the ci/vale/styles/config/vocabularies/nat/accept.txt file are OK.

Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,

and should contain an Apache License 2.0 header comment at the top of each file.

  • Confirm that copyright years are up-to date whenever a file is changed.

Files:

  • pyproject.toml
  • examples/documentation_guides/workflows/text_file_ingest/pyproject.toml
  • examples/evaluation_and_profiling/email_phishing_analyzer/pyproject.toml
  • packages/nvidia_nat_test/src/nat/test/plugin.py
  • tests/test_data/docker-compose.services.yml
  • src/nat/utils/__init__.py
  • examples/frameworks/multi_frameworks/pyproject.toml
  • packages/nvidia_nat_test/src/nat/test/utils.py
  • examples/front_ends/simple_auth/tests/test_simple_auth.py
examples/**/*

⚙️ CodeRabbit configuration file

examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.

  • If an example contains Python code, it should be placed in a subdirectory named src/ and should
    contain a pyproject.toml file. Optionally, it might also contain scripts in a scripts/ directory.
  • If an example contains YAML files, they should be placed in a subdirectory named configs/. - If an example contains sample data files, they should be placed in a subdirectory named data/, and should
    be checked into git-lfs.

Files:

  • examples/documentation_guides/workflows/text_file_ingest/pyproject.toml
  • examples/evaluation_and_profiling/email_phishing_analyzer/pyproject.toml
  • examples/frameworks/multi_frameworks/pyproject.toml
  • examples/front_ends/simple_auth/tests/test_simple_auth.py
**/*.py

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible

Files:

  • packages/nvidia_nat_test/src/nat/test/plugin.py
  • src/nat/utils/__init__.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
  • examples/front_ends/simple_auth/tests/test_simple_auth.py
{src/**/*.py,packages/*/src/**/*.py}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

{src/**/*.py,packages/*/src/**/*.py}: All importable Python code must live under src/ or packages//src/
All public APIs must have Python 3.11+ type hints on parameters and return values
Prefer typing/collections.abc abstractions (e.g., Sequence over list)
Use typing.Annotated for units or metadata when useful
Treat pyright warnings as errors during development

Files:

  • packages/nvidia_nat_test/src/nat/test/plugin.py
  • src/nat/utils/__init__.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
packages/*/src/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

If a package contains Python code, it must have tests in a tests/ directory at the same level as pyproject.toml

Files:

  • packages/nvidia_nat_test/src/nat/test/plugin.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
packages/**/*

⚙️ CodeRabbit configuration file

packages/**/*: - This directory contains optional plugin packages for the toolkit, each should contain a pyproject.toml file. - The pyproject.toml file should declare a dependency on nvidia-nat or another package with a name starting
with nvidia-nat-. This dependency should be declared using ~=<version>, and the version should be a two
digit version (ex: ~=1.0).

  • Not all packages contain Python code, if they do they should also contain their own set of tests, in a
    tests/ directory at the same level as the pyproject.toml file.

Files:

  • packages/nvidia_nat_test/src/nat/test/plugin.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
src/nat/**/*

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

src/nat/**/* contains core functionality; changes should prioritize backward compatibility

Files:

  • src/nat/utils/__init__.py

⚙️ CodeRabbit configuration file

This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.

Files:

  • src/nat/utils/__init__.py
🧠 Learnings (5)
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to packages/*/pyproject.toml : packages/*/pyproject.toml must declare a dependency on nvidia-nat or a package starting with nvidia-nat-

Applied to files:

  • examples/documentation_guides/workflows/text_file_ingest/pyproject.toml
  • examples/evaluation_and_profiling/email_phishing_analyzer/pyproject.toml
  • examples/frameworks/multi_frameworks/pyproject.toml
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to packages/*/pyproject.toml : Dependencies in pyproject.toml should use ~=<version> with two-digit versions (e.g., ~=1.0)

Applied to files:

  • examples/documentation_guides/workflows/text_file_ingest/pyproject.toml
  • examples/evaluation_and_profiling/email_phishing_analyzer/pyproject.toml
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py} : Extract frequently repeated test code into pytest fixtures

Applied to files:

  • packages/nvidia_nat_test/src/nat/test/plugin.py
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py} : Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints

Applied to files:

  • packages/nvidia_nat_test/src/nat/test/plugin.py
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {docs/source/**/*.md,**/README.@(md|ipynb)} : Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)

Applied to files:

  • examples/frameworks/multi_frameworks/pyproject.toml
🧬 Code graph analysis (3)
src/nat/utils/__init__.py (2)
src/nat/builder/workflow_builder.py (1)
  • build (263-378)
src/nat/runtime/session.py (1)
  • SessionManager (46-219)
packages/nvidia_nat_test/src/nat/test/utils.py (1)
src/nat/utils/__init__.py (1)
  • run_workflow (28-76)
examples/front_ends/simple_auth/tests/test_simple_auth.py (4)
src/nat/authentication/oauth2/oauth2_auth_code_flow_provider_config.py (1)
  • OAuth2AuthCodeFlowProviderConfig (22-40)
src/nat/data_models/authentication.py (2)
  • AuthenticatedContext (66-77)
  • AuthFlowType (53-63)
tests/nat/front_ends/auth_flow_handlers/mock_oauth2_server.py (3)
  • token_url (143-144)
  • authorization_url (140-141)
  • token (250-274)
packages/nvidia_nat_test/src/nat/test/utils.py (1)
  • locate_example_config (57-68)
🪛 Ruff (0.14.3)
packages/nvidia_nat_test/src/nat/test/plugin.py

646-646: Consider moving this statement to an else block

(TRY300)


647-647: Do not catch blind exception: Exception

(BLE001)


650-650: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


712-718: Consider moving this statement to an else block

(TRY300)


720-720: Do not catch blind exception: Exception

(BLE001)


723-723: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

🔇 Additional comments (14)
examples/evaluation_and_profiling/email_phishing_analyzer/pyproject.toml (1)

14-14: LGTM! Correct dependency update.

The migration from bs4==0.0.2 to beautifulsoup4~=4.13 is appropriate. The bs4 package is deprecated, and beautifulsoup4 is the correct package name for Beautiful Soup 4. The version specifier follows the coding guidelines by using ~= with a two-digit version.

examples/documentation_guides/workflows/text_file_ingest/pyproject.toml (1)

14-14: LGTM! Correct dependency update.

The migration from bs4==0.0.2 to beautifulsoup4~=4.13 is appropriate, replacing the deprecated package name with the correct one.

src/nat/utils/__init__.py (1)

70-76: LGTM! Clean session-based workflow execution.

The refactor to use SessionManager with explicit session scope is well-structured. Normalizing session_kwargs to an empty dict when None is a clean pattern, and forwarding it to the session enables custom authentication and other session-level configuration.

pyproject.toml (1)

226-226: LGTM! Appropriate dev dependency addition.

Adding beautifulsoup4~=4.13 to the dev dependency group supports the new OAuth2 test fixtures that parse HTML responses. The version specifier follows coding guidelines.

tests/test_data/docker-compose.services.yml (1)

167-181: LGTM! OAuth2 test service properly configured.

The new oauth2-server service is well-configured for integration testing:

  • AUTHLIB_INSECURE_TRANSPORT=1 appropriately allows HTTP for test environments
  • Port mapping 5001:5000 avoids conflicts
  • Healthcheck validates service availability before tests run
  • Build context correctly points to the simple_auth example
.gitlab-ci.yml (2)

56-57: LGTM! OAuth2 CI variables properly defined.

The environment variables correctly point to the OAuth2 service that will be available in the test stage, using the service alias and internal container port.


140-146: Verify the OAuth2 server image is built and pushed to the registry.

The service references $CI_REGISTRY_IMAGE/oauth2_server, but there's no apparent build stage in this CI configuration that creates and pushes this image to the registry.

Run the following script to check if there's a separate CI job or workflow that builds the OAuth2 server image:

examples/frameworks/multi_frameworks/pyproject.toml (1)

15-16: LGTM! Correct dependency update.

The migration from bs4==0.0.2 to beautifulsoup4~=4.13 is appropriate. The dependency reordering maintains alphabetical sorting.

packages/nvidia_nat_test/src/nat/test/plugin.py (2)

693-710: HTML parsing is fragile but acceptable for test fixtures.

The HTML parsing logic (lines 693-710) relies on the specific structure of the OAuth2 server's HTML response. This could break if the HTML format changes. However, for test fixtures where both the server and client are controlled within the same repository, this trade-off is acceptable.

If the HTML structure changes frequently, consider having the OAuth2 server provide a JSON API endpoint for client creation as an alternative to HTML scraping.


632-651: Add return type hint for the fixture.

The function is missing a return type hint. Python 3.11+ type hints are required for all public APIs per coding guidelines.

Apply this diff:

 @pytest.fixture(name="oauth2_server_url", scope="session")
-def oauth2_server_url_fixture(fail_missing: bool) -> str:
+def oauth2_server_url_fixture(fail_missing: bool) -> str:
     """
     To run these tests, an oauth2 server must be running.
     """

Wait, I see the return type IS present (-> str on line 633). Let me re-check... Yes, the return type is there. So this is fine.

LGTM! OAuth2 server URL fixture follows established patterns.

The fixture correctly follows the same pattern as other service fixtures in this file (etcd_url, phoenix_url, etc.), checking connectivity and skipping or failing based on the fail_missing flag.

⛔ Skipped due to learnings
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py} : Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
examples/front_ends/simple_auth/tests/test_simple_auth.py (3)

1-14: LGTM: License header is correct.

The SPDX Apache-2.0 header with 2025 copyright is properly formatted.


76-81: Investigate and resolve type mismatch instead of using type: ignore.

The type: ignore[arg-type] comment suggests a type mismatch that should be properly resolved. Please investigate the underlying type issue and fix it rather than suppressing the warning.

Can you verify what type mismatch is being suppressed here and whether it can be resolved by proper typing or if there's a legitimate reason to ignore it? If legitimate, add a comment explaining why.


104-132: LGTM: Test logic is well-structured.

The integration test appropriately:

  • Configures environment variables with test credentials
  • Overrides OAuth endpoints to point to the test server
  • Uses functools.partial to bind test cookies to the auth callback
  • Validates the workflow result against expected credentials

The use of session_kwargs to inject the authentication callback is clean and aligns well with the PR's objective to support custom session parameters.

packages/nvidia_nat_test/src/nat/test/utils.py (1)

84-84: LGTM: Correctly forwards kwargs to underlying function.

The addition of **kwargs forwarding enables tests to pass additional parameters like session_kwargs to the underlying workflow execution, which aligns with the PR objective to support custom session parameters.

@dagardner-nv
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 9d97720 into NVIDIA:develop Nov 5, 2025
16 of 17 checks passed
@dagardner-nv dagardner-nv deleted the david-simple-auth-e2e branch November 5, 2025 16:08
saglave pushed a commit to snps-scm13/SNPS-NeMo-Agent-Toolkit that referenced this pull request Dec 11, 2025
* Add the ability to optionally pass session keyword arguments with `run_workflow`
* Add Oauth example service to `tests/test_data/docker-compose.services.yml`
* Change Beautiful Soup dependency to use the `beautifulsoup4` name and not the deprecated `bs4` package name.

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/NeMo-Agent-Toolkit/blob/develop/docs/source/resources/contributing.md).
- We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
  - Any contribution which contains commits that are not Signed-Off will not be accepted.
- When the PR is ready for review, new or existing tests cover these changes.
- When the PR is ready for review, the documentation is up to date with these changes.

## Summary by CodeRabbit

## Release Notes

* **Tests**
  * Introduced OAuth2 Authorization Code flow testing infrastructure with new fixtures for server connectivity and client credential management.

* **New Features**
  * Added session configuration support to workflow execution through new optional parameters.

* **Chores**
  * Updated HTML parsing dependencies to current versions across packages.

Authors:
  - David Gardner (https://github.com/dagardner-nv)

Approvers:
  - https://github.com/Salonijain27
  - Eric Evans II (https://github.com/ericevans-nv)

URL: NVIDIA#1148
Signed-off-by: Sangharsh Aglave <aglave@synopsys.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement to existing functionality non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants