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

Is snapshots working good ? #118

Open
EvoXCX opened this issue Jul 23, 2024 · 7 comments
Open

Is snapshots working good ? #118

EvoXCX opened this issue Jul 23, 2024 · 7 comments

Comments

@EvoXCX
Copy link

EvoXCX commented Jul 23, 2024

Hello,
I have a problem with creating snapshots with PyAPTly, it raise an error say I do not respect the syntax, nothing wrong with my configuration file as it's the same as provided by examples.

For example my bookworm snapshots from existing mirrors I created.

[snapshot."bookworm-main-%D"]
mirror = "bookworm-main" 

[snapshot."bookworm-updates-%D"]
mirror = "bookworm-updates"

[snapshot."bookworm-security-%D"]
mirror = "bookworm-security"

[snapshot."docker-bookworm-%D"]
mirror = "docker-bookworm" 

[snapshot."postgres-bookworm-%D"]
mirror = "postgres-bookworm"

[snapshot."php-bookworm-%D"]
mirror = "php-bookworm"

Command issued: pyaptly snapshot snapshots_bookworm.toml create

Here is the full error with Traceback in case of this issue need to go further:

Traceback (most recent call last):
  File "/opt/pyaptly_venv/lib/python3.11/site-packages/pyaptly/cli.py", line 30, in entry_point
    cli.main(argv[1:])
  File "/opt/pyaptly_venv/lib/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/opt/pyaptly_venv/lib/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/pyaptly_venv/lib/python3.11/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/pyaptly_venv/lib/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/pyaptly_venv/lib/python3.11/site-packages/pyaptly/cli.py", line 143, in snapshot
    cfg = main.prepare(fake_args)
          ^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/pyaptly_venv/lib/python3.11/site-packages/pyaptly/main.py", line 45, in prepare
    cfg = tomli.load(f)
          ^^^^^^^^^^^^^
  File "/opt/pyaptly_venv/lib/python3.11/site-packages/tomli/_parser.py", line 66, in load
    return loads(s, parse_float=parse_float)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/pyaptly_venv/lib/python3.11/site-packages/tomli/_parser.py", line 113, in loads
    pos, header = create_dict_rule(src, pos, out)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/pyaptly_venv/lib/python3.11/site-packages/tomli/_parser.py", line 298, in create_dict_rule
    raise suffixed_err(src, pos, "Expected ']' at the end of a table declaration")
tomli.TOMLDecodeError: Expected ']' at the end of a table declaration (at line 1, column 25)
(pyaptly_venv) root@srv-deb-repo:/home/user/PyAPTly Configuration/snapshots# pyaptly snapshot snapshots_bullseye.toml create
ERROR: Expected ']' at the end of a table declaration (at line 1, column 25)
               Wrote traceback to: /tmp/tmpe016cvzz

Thanks in advance for your help 😄

@Melkor333
Copy link
Contributor

The error indicates an error in your toml config file.

Expected ']' at the end of a table declaration (at line 1, column 25)

This Error says that the first line is wrong, after the 25th character, it expects a ] (I assume the 25. character is a "?).
The snippet you posted is AFAIU only a part of your whole config, right? At least with the snippet, the parser doesn't throw an error when I test it.

Otherwise can you post the very first line of your configfile?

It might also help to put your toml config through something such as toml-lint to make sure it's valid TOML.

@EvoXCX
Copy link
Author

EvoXCX commented Jul 24, 2024

Hello @Melkor333,

%D is not supported apparently by pyaptly, snapshots works with a "normal" name.
All my snapshot configuration is in the upper issue, I use separate files to do what I need.

@Melkor333
Copy link
Contributor

Hello @Melkor333,

%D is not supported apparently by pyaptly, snapshots works with a "normal" name. All my snapshot configuration is in the upper issue, I use separate files to do what I need.

Hm strange that it already fails at parsing anyway. But yeah the only thing that works right now is %T, which includes some manual parsing IIRC.

You'd like to only have the day with %D, right?
That may be something to put in the backlog -> Allow all kinds of Date/Time formats...

@EvoXCX
Copy link
Author

EvoXCX commented Jul 24, 2024

Yeah, this is exactly what I want to do.

@rhizoome
Copy link

rhizoome commented Aug 1, 2024

%D won't do anything for you. %T is a feature implemented in pyaptly. It means that you get snapshots with their retention managed by pyaptly. I guess it was a bad idea to name it %T, as it suggests other known %X placeholders will work.

But I was confused, why you get a parse error. You should get a snapshot that is literally named bookworm-main-%D. I tried to reproduce this problem. Exactly what I described happened, I cannot reproduce. Did you install the exact version defined in poetry.lock? No need to actually invest time into this, as you'll just get a snapshot that is literally named bookworm-main-%D. I just mention this for future reference.

As for supporting %D is don't see a reason to do that. How the timestamps work will never be selectable by the user, at it is deeply connected to the retention algorithm. We could format the timestamp different, but that opens the door for many bugs, for no apparent advantage.

@Melkor333
Copy link
Contributor

Melkor333 commented Aug 12, 2024

Fair argument against generalizing %T! maybe a "solution" would be to completely hide this from the user (and therefore also eliminate configuration errors) by having an additional flag like:

[snapshot."bookworm-main"]
mirror = "bookworm-main" 
add_timestamp = true

That would also mean we don't need to parse the snapshot name to see if it has timestamps.

@rhizoome
Copy link

That would also mean we don't need to parse the snapshot name to see if it has timestamps.

That is a great proposal!

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

No branches or pull requests

3 participants