Update connection names as instance IPs change#269
Merged
Conversation
This is only done if: - The instance already exists in the database - The instance is in a state that would cause it to be added to the database if it did not already exist there
github-actions
Bot
added
version bump
12 tasks
In SQL strings should be enclosed in single quotes (in case they contain whitespace) but numeric value must not.
Quotes do not seem to be necessary at all when using psycopg.sql.SQL().
This avoids using the same cursor for the connection ID query and the updating of the connection names, which generates an error.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates guacscanner so that when an EC2 instance already has a Guacamole connection, the connection name is refreshed to reflect changes in the instance’s IP-related metadata.
Changes:
- Add a SQL query and helper functions to update
guacamole_connection.connection_namefor existing connections. - Invoke the update logic when an instance is in an “add” state but its connection already exists.
- Bump the package version.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/guacscanner/guacscanner.py |
Adds update query + functions and calls them for existing connections to refresh connection names. |
src/guacscanner/_version.py |
Bumps module version to 3.0.1-rc.4. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
In Postgres, even setting the same value generally creates a new row version, generating table bloat. Adding this predicate avoids no-op updates
update_connection_name() opens a new cursor and commits the transaction each iteration. In psycopg/PostgreSQL, committing will close/invalidate any open cursors (including the one being iterated), which can raise errors mid-loop and/or lead to partial updates. By fetching all connection_ids first, then performing updates without committing inside the loop, we avoid this problem.
This was referenced Apr 28, 2026
9 tasks
jsf9k
added a commit
to cisagov/guacscanner-docker
that referenced
this pull request
Apr 29, 2026
This also necessitates relocking the Pipfile. This is possible now that cisagov/guacscanner#269 has been merged and a new release created.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🗣 Description
This pull request modifies the Python code to update connection names as instances' internal and external IPs change.
See also:
cisagov/guacscannerguacscanner-docker#42cisagov/guacamole-compositionansible-role-guacamole#88cisagov/ansible-role-guacamoleguacamole-packer#124💭 Motivation and context
Resolves #268.
🧪 Testing
All automated tests pass. I also deployed a new cisagov/guacamole-packer AMI with this change as part of the testing for cisagov/guacamole-packer#124 and verified that it functions as expected.
✅ Pre-approval checklist
bump_versionscript if this repository is versioned and the changes in this PR warrant a version bump.✅ Pre-merge checklist
✅ Post-merge checklist