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

Jump to bottom

MAINT: Adding Identifiers to Memory #145

Merged
merged 5 commits into from
Apr 18, 2024
Merged

MAINT: Adding Identifiers to Memory #145

merged 5 commits into from
Apr 18, 2024

Conversation

rlundeen2
Copy link
Contributor

@rlundeen2 rlundeen2 commented Apr 16, 2024 •
edited
Loading

Description

  • Adds an identifier for ochestrator, converter, and target to be stored in the database. This is cleaner because actual objects were never stored.
  • Raises the error if there is a database error. This has made a few errors sneak by us in the past
  • Fixes bug with demo 2 (due to the object being stored in the database and error swallowed)
  • Fixes bug in howto guide
  • Adds shortcut script to squash commits
  • Fixes bugs with variation and translation converters where conversation_id was not correct.
  • Removes NoopConverter (at this point it is not needed)
  • Adds print_schema to duckdb_memory to visualize the schema

Tests and Documentation

All jupytext notebooks run

@rlundeen2 rlundeen2 changed the title Adding Identifiers to Memory [MAIN] Adding Identifiers to Memory Apr 16, 2024
@rlundeen2 rlundeen2 changed the title [MAIN] Adding Identifiers to Memory [MAINT] Adding Identifiers to Memory Apr 16, 2024
@rlundeen2 rlundeen2 marked this pull request as ready for review April 16, 2024 23:32
@rlundeen2 rlundeen2 changed the title [MAINT] Adding Identifiers to Memory MAINT: Adding Identifiers to Memory Apr 17, 2024
romanlutz
romanlutz approved these changes Apr 17, 2024
View reviewed changes
Copy link
Contributor

@romanlutz romanlutz left a comment

Choose a reason for hiding this comment

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

Thanks for addressing so many of the things we talked about the past weeks 🙂

.github/squash_commits.ps1 Outdated Show resolved Hide resolved
doc/demo/2_multiturn_strategies.ipynb Outdated Show resolved Hide resolved
pyrit/memory/duckdb_memory.py Show resolved Hide resolved
pyrit/memory/memory_models.py Show resolved Hide resolved
pyrit/models/identifiers.py Outdated Show resolved Hide resolved
pyrit/orchestrator/orchestrator_class.py Outdated Show resolved Hide resolved
romanlutz
romanlutz reviewed Apr 17, 2024
View reviewed changes
doc/how_to_guide.py Outdated Show resolved Hide resolved
cseifert1
cseifert1 approved these changes Apr 17, 2024
View reviewed changes
Copy link
Contributor

@cseifert1 cseifert1 left a comment

Choose a reason for hiding this comment

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

Thank you for making another change to ensure the plumbing all works. I see the DB view in the memory.py/ipynb is gone. Would be great to add this back in as I found it very informative to understand the DB structure.

pyrit/models/prompt_request_piece.py Show resolved Hide resolved
pyrit/memory/memory_interface.py Show resolved Hide resolved
pyrit/prompt_converter/prompt_converter.py Show resolved Hide resolved
pyrit/prompt_target/prompt_target.py Show resolved Hide resolved
@rlundeen2 rlundeen2 merged commit 2775b76 into Azure:main Apr 18, 2024
4 checks passed
jbolor21 pushed a commit to jbolor21/jbolor-PyRIT that referenced this pull request Apr 22, 2024
@rlundeen2 @jbolor21
MAINT: Adding Identifiers to Memory (Azure#145)
df0d459 
* Adding Identifiers to Memory

* Adding squash commit shortcut

* reverting notebook

* PR feedback

* PR feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romanlutz romanlutz romanlutz approved these changes

@cseifert1 cseifert1 cseifert1 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rlundeen2 @cseifert1 @romanlutz