#1125 Allow non-credentialed users to run ETL pipelines #1150
Conversation
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Contributor
Author
|
It looks like the PR Checks Backend are failing just because #1147 isn't in yet. I want to double check things (there's one Slack message I may have missed) and try this out on different computers before switching it from a draft. |
cfreedman
approved these changes
Apr 1, 2025
Contributor
|
This PR has been marked as stale because it has been open for 7 days with no activity. |
adamzev
force-pushed
the
1125-credential-handling
branch
2 times, most recently
from
c81349f to
db1890a
Compare
# 1.0.0 (2025-04-06) ### Bug Fixes * **572:** fix webpack warnings + fix linting issues ([CodeForPhilly#574](https://github.com/adamzev/clean-and-green-philly/issues/574)) ([fbfd459](fbfd459)) * Add focus styling to header links ([a99065c](a99065c)) * Add href to dropdown items to increase link size ([2eb4667](2eb4667)) * closing tooltip popup on map closes property details ([432e8c0](432e8c0)) * correct the scrolling on the map page ([a850237](a850237)) * Remove nested link and button ([d93e8bd](d93e8bd)) * Update footer markup structure for accessibility ([d5c2a33](d5c2a33)) ### Features * **247:** make the mapbox legend more legible ([bcf6915](bcf6915)) * **293:** sort features by priority of high, medium, low ([8cd1058](8cd1058)) * **305:** expand the header width of single property table ([b25261e](b25261e)) * **395:** add white background to backbutton bar ([a65f052](a65f052)) * **395:** make property selection back button sticky ([6601201](6601201)) * **457:** add filter count to button ([b35145a](b35145a)) * **535:** add close download view button ([CodeForPhilly#579](https://github.com/adamzev/clean-and-green-philly/issues/579)) ([9bd2c53](9bd2c53)) * **544:** adjust remove property section ([CodeForPhilly#580](https://github.com/adamzev/clean-and-green-philly/issues/580)) ([900af8e](900af8e)) * **568:** restyle the property table ([CodeForPhilly#577](https://github.com/adamzev/clean-and-green-philly/issues/577)) ([2c8cc92](2c8cc92)) * Add arrow icon to link ([6bce3fa](6bce3fa)) * Add content to sidebar ([23d5c52](23d5c52)) * Add council district ([e576218](e576218)) * Add extra data to single detail page ([48e34fe](48e34fe)) * Add font ([c1a8b93](c1a8b93)) * Add font to map ([1491518](1491518)) * Add font to search field ([3b08069](3b08069)) * Add info cards ([8170944](8170944)) * Add missing data and atlas link; style table ([8c260d4](8c260d4)) * Add percentage, update column widths ([41caf77](41caf77)) * Add priority color square ([ad5a9fa](ad5a9fa)) * add redirect from map to find-properties ([439ed6c](439ed6c)) * Add row scope ([f945979](f945979)) * Add spacing below Back button ([7de48c5](7de48c5)) * Address a few more links ([b3b6733](b3b6733)) * Adjust table per requirements ([36e6cfa](36e6cfa)) * Change Find Properties and Learn More to links ([1de0b47](1de0b47)) * Close sidebar if clicking on empty map ([612117d](612117d)) * Navigate to map on click; show popups ([183b6a2](183b6a2)) * Open sidebar from map ([5474367](5474367)) * Pass selected property to map view ([663f142](663f142)) * underline links ([5f23d09](5f23d09)) * Update aria label and role ([765e5a4](765e5a4))
Allow creating db url by specifing port and password (to be used in Cloud Run to avoid duplicate secret management of POSTGRES_PASSWORD and VACANT_LOTS_DB) Add additional logic to psql to user localhost in Cloud Run environments Add a new bucket manager that should work in Cloud Run, with file credentials and in read-only mode for non-credentialed users Changed slack reporters to not report with a message rather than erroring if no credentials exist
Print rather than error is Slack credentials are missing
adamzev
force-pushed
the
1125-credential-handling
branch
from
db1890a to
7167ebc
Compare
adamzev
commented
Apr 26, 2025
data/src/config/psql.py
Outdated
|
|
||
| if os.getenv("VACANT_LOTS_DB"): | ||
| # Use the provided database URL | ||
| url = os.getenv("VACANT_LOTS_DB") |
Contributor
Author
There was a problem hiding this comment.
I probably should switch this back to
os.environ["VACANT_LOTS_DB"].replace("localhost", "host.docker.internal")
if is_docker()
else os.environ["VACANT_LOTS_DB"]
in-case we're in docker and VACANT_LOTS_DB is defined.
adamzev
commented
Apr 26, 2025
| sys.path.append(os.path.dirname(os.path.abspath(__file__)) + "/..") | ||
|
|
||
| from classes.slack_error_reporter import ( | ||
| from new_etl.classes.slack_reporters import ( |
Contributor
Author
There was a problem hiding this comment.
Switch back to classes rather than new_etl
Contributor
Author
|
I rebased this but saw some things I need to change. I also want to expand this fix to the old ETL pipeline. |
The pipeline can be run locally but will not write changes to the bucket and will not report errors to slack without credentials. Previously, the code would error when credentials were missing. Added BACKUP_SCHEMA=False and made FORCE_RELOAD=True as the default. These are temporary fixes and will not be relevant once postgres is removed.
adamzev
changed the base branch from
staging
to
lebovits/issu717-store-streetview-imgs-in-subdirectory
adamzev
changed the base branch from
lebovits/issu717-store-streetview-imgs-in-subdirectory
to
staging
Contributor
|
This PR has been marked as stale because it has been open for 7 days with no activity. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Allow non-credentialed users to run ETL pipelines and support Application Default Credentials
feat: allow non-credentialed users and cloud env credentials
The ability to specify ports and passwords rather than a full url was added to better suit not having to duplicate secret management (POSTGRES_PASSWORD and VACANT_LOTS_DB)
Added additional logic to psql to user localhost in Cloud Run environments
Added a new bucket manager that should work in the following enviroments:
Changed Slack reporters to not report with a message rather than throwing an error if no credentials exist
Related Issue(s)
This PR addresses issue #1125
Type of change
How Has This Been Tested?
I've tried this locally with credentials (to my own bucket), without credentials and within Cloud Run. I don't have Slack credentials so I haven't tried it with them.