feat: change in Readme file and a question about the code

WaliZaman commented 1 year ago

Description

Test.py contains a test version for reading sql file and outputting a csv file. The Readme has a few changes.

Dependency Changes (delete if not applicable)

Pipfile and pipfile.lock added.

Testing

Test.py reads sql database with an already established connection and hosted locally, and outputs a csv file. Is there a way to do this with a sql file without locally hosting it?

Documentation (delete if not applicable)

Readme file contains a few changes documenting how to install and run pipenv.

Checklist

Put an x in the boxes that apply to this pull request (you can also fill these out after opening the pull request). If you're unsure about any of these, don't hesitate to leave a comment on this pull request!

[X] I have read the road86 Contribution Guide.
[X] I have checked all commit message styles match the requested structure.
[X] My code follows the style guidelines of this project.
[X] I have performed a self-review of my own code.
[X] I have assigned at least one reviewer.
[x] New and existing unit tests pass locally with my changes.
[ ] I have added tests that prove my fix is effective or that my feature works.
[X] My changes generate no new warnings.
[X] I have made corresponding changes to the documentation.
[x] New database changes have been committed.

ChasNelson1990 commented 1 year ago

P.S. I would be interested to know why you created a new PR rather than continuing to use the existing one?

ChasNelson1990 commented 1 year ago

Is there a way to do this with a sql file without locally hosting it?

Spent a little time thinking on this... the tl;dr is "no"

The long answer is... "maybe"

However, the "what we might actually want to do [some time in the future that isn't now]" answer is that maybe bahis-data should have a docker-compose config that basically sets up two docker pods - 1) PostgreSQL and 2) vanilla Python 3.10. The entry script for "1" could then be a short script that reads the file into the db and the entry script for "2" would just run the suite of pipeline files?

ChasNelson1990 commented 1 year ago

@WaliZaman I was using this as an example with Iko and realised that a major comment has gone missing :confused:...

Roughly, it said - if this is really a test we should discuss using pytest properly.

WaliZaman commented 1 year ago

@WaliZaman I was using this as an example with Iko and realised that a major comment has gone missing confused...

Roughly, it said - if this is really a test we should discuss using pytest properly.

When should we discuss this? I'll look into pytest a little as well.

WaliZaman commented 1 year ago

P.S. I would be interested to know why you created a new PR rather than continuing to use the existing one?

I thought I had to close a PR once the task was done, so I closed it and opened a new one.

ChasNelson1990 commented 1 year ago

I thought I had to close a PR once the task was done, so I closed it and opened a new one.

Nope, you generally do one PR per feature branch and you "resolve" the comments and requests on that PR as you go, rerequesting reviews as needed.

road86 / bahis-data