Features/#325 import status2019 #326
Conversation
| cts_demand_buildings, | ||
| emobility_mit, | ||
| ] | ||
| ) |
There was a problem hiding this comment.
Why do you need all these dependencies?
There was a problem hiding this comment.
These were the dependencies used for the low and medium flexibility scenarios. Since the creation concept of our new scenarios is the same, it makes sense to me to use the same dependencies. Now that you mentioned this, I think it is better to have this import status2019 and the task that creates the new scenarios in the same dataset.
There was a problem hiding this comment.
I am sorry but I don't really get what you want to change.
I think it is fine like this, it might help to make a comment in the code why these dependencies are set.
There was a problem hiding this comment.
I meant these changes: 723295a
| ) | ||
|
|
||
| my_env = os.environ.copy() | ||
| my_env["PGPASSWORD"] = "data" |
There was a problem hiding this comment.
Is there an option to get the password from somewhere else?
There was a problem hiding this comment.
Now we can retrieve the password from datasets.yml a83ca47
| "-a", | ||
| "--single-transaction", | ||
| f"--table={table}", | ||
| "data_bundle_powerd_data/PoWerD_status2019-v2.backup", |
There was a problem hiding this comment.
Can you refer to the already uploaded backup? This way we would upload the same data again, right?
| database = config_data["--database-name"] | ||
| host = config_data["--database-host"] | ||
| port = config_data["--database-port"] | ||
| user = config_data["--database-user"] |
There was a problem hiding this comment.
The password is stored in config_data as well, isn't it? At least it is listed in my config files.
There was a problem hiding this comment.
As you mentioned, a parameter called database-password is in the configuration file. But it can be modified anytime by the user, and then it would not match the fixed password that the status2019 has.
There was a problem hiding this comment.
But that is the same for all parameters, isn't it?
Other functions also access the password from there, so it would be a problem anyhow.
There was a problem hiding this comment.
If that is the case, shouldn't we take database-password out from the configuration file?
There was a problem hiding this comment.
Since I would like to merge this branch before proceeding with the scenario path creation, I will retrieve the password using the database-password parameter in the configuration file, as you suggested.
There was a problem hiding this comment.
Sorry, I completely forgot to reply. Looks good to me!
| """ | ||
| # Get parameters from config and set download URL | ||
| url = sources["url_status2019"] | ||
| status2019_path = Path(".") / "PoWerD_status2019.backup" |
There was a problem hiding this comment.
Is it okay to have different filenames than in the url? It is called PoWerD_status2019_V2.backup" there. (btw: It is not needed to call the versions of files in zenodo differently. We also don't do that for the other zenodo-uploads to reduce the code-wise changes. It is not possible to change this in your repo, I just mention it for future updates.)
There was a problem hiding this comment.
It is not a problem. The file is stored using the provided name.
Fixes #325