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

Rename columns that changed in a recent update #103

Merged
merged 1 commit into from
May 21, 2023

Conversation

CaAlden
Copy link
Contributor

@CaAlden CaAlden commented Apr 13, 2023

After a recent update to things, the database was moved and some of the columns in the TMTask table were renamed. See discussion on the ticket: #100

There is another problem that the database has also moved. The issue suggests exporting a custom THINGSDB environment variable, but because this PR does not address that issue, I don't feel comfortable marking that issue as resolved by this.


Also, hello! This is my first time contributing. Please let me know if there's anything you would like me to do to make this PR match the project's standards in some way. I didn't see a readme for contributing.

@sonarcloud
Copy link

sonarcloud bot commented Apr 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@patrick91
Copy link

Test this for my app and it works fine! thanks, I hope it will get merged 😊

@CaAlden
Copy link
Contributor Author

CaAlden commented Apr 21, 2023

@AlexanderWillner I'm not sure who the right person is to bring this up with, but the library currently will not function without these changes. Is there anything I can do to help move this along? (If you're not the correct person to ping, is there someone else I can ask?) 🙏🏻

@ishan1m
Copy link

ishan1m commented Apr 22, 2023

@CaAlden Is there any way for me to use these changes locally on things-cli? (sorry if this is a naive question 🙈)

@CaAlden
Copy link
Contributor Author

CaAlden commented Apr 23, 2023

@ishan1m

It might not be the simplest thing, but you could get something working if you checkout things-cli locally and pull in my fork anywhere things-cli is using things from pip. I have a project using things in the same way right now but I'm hoping for this to merge so I can undo the hack-y-ness.

I basically did git submodule add with my fork in my project and changed my references to things to use the local library. (When I did it this way I added an __init__.py to the submodule and changed my imports to import things from things.

@AlexanderWillner
Copy link
Contributor

Thanks - I'll make the needed changes in all repositories

@ishan1m
Copy link

ishan1m commented Apr 24, 2023

Thanks for the detailed response @CaAlden!

Thanks @AlexanderWillner, looking forward to it!

@CaAlden
Copy link
Contributor Author

CaAlden commented Apr 30, 2023

@AlexanderWillner It looks like the tests are failing because the main.sqlite file committed in the tests directory has the old column names. Is there a script or something for regenerating that file? How was it originally made?

@AlexanderWillner
Copy link
Contributor

@AlexanderWillner It looks like the tests are failing because the main.sqlite file committed in the tests directory has the old column names. Is there a script or something for regenerating that file? How was it originally made?

It's the actual test Things DB (Things is using sqlite). What is needed is to open the main.sqlite file with the current version of Things and save it again. The commands make db-from-things and make db-to-things were supposed to do this semi-automatically, however, the location of the database now contains a random value as well.

All can be figured out, just needs a few minutes/hours...

@mikez
Copy link
Contributor

mikez commented May 1, 2023

@CaAlden Thank you for the heads-up and the fast pull request. Also, thank you for your patience these weeks until we got back to you.

An important note (also, /cc @AlexanderWillner): we might need to check what Things version the user has; it seems not all are automatically upgraded. I'm on Big Sur (which is still actively supported) and Things is at Version 3.15.16 and thingsapi works as expected. Some of us are not on Monterey or Ventura, since Apple doesn't offer OS upgrades for some MacBooks. See also: https://endoflife.date/macos

For comparison (on Big Sur with Version 3.15.16):

>>> import things
>>> things.Database().get_version()
21

The SQL query might be formed conditionally based on database version.

@mikez
Copy link
Contributor

mikez commented May 2, 2023

Follow-up and new idea. Instead of implementing backwards-compatible support, do this:

  • check the version number of the Things Database (things.Database().get_version()).
  • If it is below a certain number, then prompt the user to install an older version of thingsapi; e.g. pip install things.py==0.0.14.

That keeps the code clean.

mltucker added a commit to amazingmarvin/things.sh that referenced this pull request May 8, 2023
See thingsapi/things.py#103

Thanks to Phil R.!
mltucker added a commit to amazingmarvin/things.sh that referenced this pull request May 8, 2023
See thingsapi/things.py#103

Thanks to Phil R.!
mltucker added a commit to amazingmarvin/things.sh that referenced this pull request May 8, 2023
See thingsapi/things.py#103

Thanks to Phil R.!
AlexanderWillner pushed a commit to AlexanderWillner/things.sh that referenced this pull request May 10, 2023
@roelvangils
Copy link

roelvangils commented May 14, 2023

I looks like the problem is solved. I hope that non-developers like myself can soon use Things CLI again by simply upgrading the pip package! 🤞 I'm on macOS 13.3.1 (a) (22E772610a) and Things 3.17.7 (31707500).

@AlexanderWillner
Copy link
Contributor

  • check the version number of the Things Database (things.Database().get_version()).
  • If it is below a certain number, then prompt the user to install an older version of thingsapi; e.g. pip install things.py==0.0.14.

Do we know in which DB version the changes happened? We could start with using version 21 as the breaking point...

@mikez
Copy link
Contributor

mikez commented May 14, 2023

  • check the version number of the Things Database (things.Database().get_version()).
  • If it is below a certain number, then prompt the user to install an older version of thingsapi; e.g. pip install things.py==0.0.14.

Do we know in which DB version the changes happened? We could start with using version 21 as the breaking point...

Sounds good to me. We can always fix it later if needed.

@mikez
Copy link
Contributor

mikez commented May 19, 2023

@AlexanderWillner Ping ;)

@AlexanderWillner AlexanderWillner merged commit c0990a8 into thingsapi:main May 21, 2023
0 of 5 checks passed
@AlexanderWillner AlexanderWillner mentioned this pull request May 21, 2023
3 tasks
@AlexanderWillner
Copy link
Contributor

Partly solved, thanks for the contributions! Next step(s) to be discussed in #110

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants