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

fix: 'archivy init' does not purge old configuration before interactive init #164

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

srevinsaju
Copy link
Member

archivy does not reset the configuration before creating the new user.
This is because archivy calls click.create_admin before resetting the configuration.
It is also recommended to purge the configuration, (and not replace existing conf), to
remove old entries

along with this patch, a new value, VERSION would be added to the configuration
which would be useful in future to migrate old configuration to new configuration tree.

…ve init

archivy does not reset the configuration before creating the new user.
This is because archivy calls click.create_admin before resetting the configuration.
It is also recommended to purge the configuration, (and not replace existing conf), to
remove old entries

along with this patch, a new value, VERSION would be added to the configuration
which would be useful in future to migrate old configuration to new configuration tree.
@codecov
Copy link

codecov bot commented Dec 31, 2020

Codecov Report

Merging #164 (1d271fd) into master (3dfb0cc) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #164      +/-   ##
==========================================
+ Coverage   81.22%   81.25%   +0.03%     
==========================================
  Files          27       27              
  Lines        1704     1707       +3     
==========================================
+ Hits         1384     1387       +3     
  Misses        320      320              
Impacted Files Coverage Δ
archivy/cli.py 71.27% <100.00%> (+0.30%) ⬆️
archivy/config.py 92.00% <100.00%> (+0.69%) ⬆️

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 2e98de1...be52745. Read the comment docs.

@srevinsaju srevinsaju marked this pull request as draft December 31, 2020 08:36
…t in archivy init

previously, archivy, when running 'archivy init' never asked the user if
they would like to remove the old users, i.e db.json. It would be useful
to ask the user if they would like to remove them
@srevinsaju srevinsaju marked this pull request as ready for review December 31, 2020 09:11
remove_old_users = click.confirm("Found an existing user database. Do you want "
"to remove them?")
if remove_old_users:
users_db.unlink()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would also remove other db objects. Maybe we can just inform them there are other created users and provide a delete-user command?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What other information can the db contain?


class Config(object):
"""Configuration object for the application"""

VERSION = VERSION
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can directly define it as VERSION = 1 here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, agreed.

@Uzay-G
Copy link
Member

Uzay-G commented Jan 21, 2021

Sorry, posted that at the wrong place 😅

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

Successfully merging this pull request may close these issues.

None yet

2 participants