Adds the signals sync script here #171
Conversation
| @@ -12,7 +12,7 @@ | |||
|
|
|||
| SQLALCHEMY_DATABASE_URI = os.environ.get( | |||
| "SQLALCHEMY_DATABASE_URI", | |||
| "mysql+mysqlconnector://root:Njcnth_1234@localhost:3306/mysql_database", | |||
| "mysql+mysqlconnector://root:ROOT_PASSWORD@localhost:3306/mysql_database", | |||
There was a problem hiding this comment.
@dmytrotsko You can switch this back if you want, but maybe we want to get this from the local .env during development?
There was a problem hiding this comment.
Yeah, good call. Will change this to get credentials from .env file.
|
@dmytrotsko I've tested most of this in staging and it seems to work. The script doesn't seem to output anything, but I was able to observe the associated DB query. I've also tested running it from Cronicle. Let me know if you think we should go another way, but this seems like it will work, and we are more likely to keep the script updated in this repo vs. delphi-admin. (Sorry about adding the package sort to this PR! I only added |
korlaxxalrok
requested review from
dmytrotsko
and removed request for
dmytrotsko
|
@melange396 Tagging you here because you had some potential changes you wanted made in https://github.com/cmu-delphi/delphi-admin/pull/205. |
|
cool, ill copy my comments from the other PR into here |
| race_breakdown = sqlalchemy.Column(sqlalchemy.Integer) | ||
| reporting_cadence = sqlalchemy.Column(sqlalchemy.String) | ||
| restrictions = sqlalchemy.Column(sqlalchemy.Text) | ||
| severenity_pyramid_rungs = sqlalchemy.Column(sqlalchemy.String) |
There was a problem hiding this comment.
| severenity_pyramid_rungs = sqlalchemy.Column(sqlalchemy.String) | |
| severity_pyramid_rungs = sqlalchemy.Column(sqlalchemy.String) |
you were talking about fixing this... is this something we can do now, or should we wait so we can change the spelling in other places at the same time?
There was a problem hiding this comment.
@melange396 Oh no that happened again. Sorry for that 😢
BUT, we may not want to apply this change right now because I've not merged PR with fix in the signal-documentation repo, so this field actually exists in signal-documentation's database 🤡
| elif len(date) == 8: | ||
| formated_date = datetime.strptime(date, self.year_month_day_date_format) |
There was a problem hiding this comment.
maybe put an else: Exception() here? maybe just log an error?
| @@ -12,7 +12,7 @@ | |||
|
|
|||
| SQLALCHEMY_DATABASE_URI = os.environ.get( | |||
| "SQLALCHEMY_DATABASE_URI", | |||
| "mysql+mysqlconnector://root:Njcnth_1234@localhost:3306/mysql_database", | |||
| "mysql+mysqlconnector://root:ROOT_PASSWORD@localhost:3306/mysql_database", | |||
There was a problem hiding this comment.
Yeah, good call. Will change this to get credentials from .env file.
This PR makes it so we can build this script into the app container image and ultimately call it from there via some remote mechanism.
Reasons:
Changes here:
sync-signals-with-metadata.pyin a place where it will be built into the web application container imageThere has been some discussion in a different repo on a few changes we might want to make. Gonna put a link to those here: https://github.com/cmu-delphi/delphi-admin/pull/205