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.

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

Remove override of AIRFLOW__CORE__SQL_ALCHEMY_CONN #1705

Merged
merged 27 commits into from
Jan 15, 2020

Conversation

greenape
Copy link
Member

Closes #1703 closes #1704

I have:

  • Formatted any Python files with black
  • Brought the branch up to date with master
  • Added any relevant Github labels
  • Added tests for any new additions
  • Added or updated any relevant documentation
  • Added an Architectural Decision Record (ADR), if appropriate
  • Added an MPLv2 License Header if appropriate
  • Updated the Changelog

Description

Removes the override of AIRFLOW__CORE__SQL_ALCHEMY_CONN from the entrypoint script, and mandates that it be provided instead of constructing it from several env vars and mandating that it be a Postgres db.

@greenape greenape added security FlowETL ready-to-merge Label indicating a PR is OK to automerge labels Dec 16, 2019
@cypress
Copy link

cypress bot commented Dec 16, 2019



Test summary

57 0 0 0


Run details

Project FlowAuth
Status Passed
Commit 2bf7270
Started Jan 15, 2020 12:19 PM
Ended Jan 15, 2020 12:21 PM
Duration 02:34 💡
OS Linux Debian - 8.11
Browser Electron 78

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@codecov
Copy link

codecov bot commented Dec 16, 2019

Codecov Report

Merging #1705 into master will decrease coverage by 19.38%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1705       +/-   ##
===========================================
- Coverage   95.27%   75.88%   -19.39%     
===========================================
  Files         188      188               
  Lines        8332     8332               
  Branches      789      789               
===========================================
- Hits         7938     6323     -1615     
- Misses        282     1815     +1533     
- Partials      112      194       +82
Flag Coverage Δ
#autoflow_unit_tests 92.91% <ø> (ø) ⬆️
#flowapi_unit_tests 83.97% <ø> (ø) ⬆️
#flowauth_unit_tests 93.65% <ø> (ø) ⬆️
#flowclient_unit_tests 78.78% <ø> (ø) ⬆️
#flowetl_unit_tests 97.35% <ø> (ø) ⬆️
#flowkit_jwt_generator_unit_tests 100% <ø> (ø) ⬆️
#flowmachine_unit_tests ?
#integration_tests 69.56% <ø> (ø) ⬆️
Impacted Files Coverage Δ
...e/flowmachine/core/sqlalchemy_table_definitions.py 0% <0%> (-100%) ⬇️
...e/flowmachine/features/subscriber/handset_stats.py 22.5% <0%> (-77.5%) ⬇️
...owmachine/features/utilities/feature_collection.py 23.8% <0%> (-76.2%) ⬇️
...es/subscriber/contact_reference_locations_stats.py 25.92% <0%> (-74.08%) ⬇️
...flowmachine/features/subscriber/distance_series.py 28.57% <0%> (-71.43%) ⬇️
...machine/features/subscriber/interevent_interval.py 28.57% <0%> (-71.43%) ⬇️
flowmachine/flowmachine/core/geotable.py 28.57% <0%> (-71.43%) ⬇️
...wmachine/features/subscriber/contact_reciprocal.py 29.31% <0%> (-70.69%) ⬇️
...ne/features/subscriber/per_location_event_stats.py 30.3% <0%> (-69.7%) ⬇️
...hine/flowmachine/features/subscriber/mds_volume.py 33.33% <0%> (-66.67%) ⬇️
... and 80 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fbb5ae...2bf7270. Read the comment docs.

@greenape greenape changed the title Remove override of AIRFLOW__CORE__SQL_ALCHEMY_CONN Remove override of AIRFLOW__CORE__SQL_ALCHEMY_CONN Jan 14, 2020
@mergify mergify bot merged commit fc996b7 into master Jan 15, 2020
@mergify mergify bot deleted the disambiguate-env-vars branch January 15, 2020 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FlowETL ready-to-merge Label indicating a PR is OK to automerge security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FlowETL should use a distinctly named password env var/secret
3 participants