Add colon (:) to invalid char for revision name

[PatilHrushikesh]

FIXES #1540

Description

As colon is used in revision range, it shouldn't be used in revision name.

Checklist

This pull request is:

• A documentation / typographical error fix
  • Good to go, no issue or tests are needed
• A short code fix
  • please include the issue number, and create an issue if none exists, which
    must include a complete example of the issue. one line code fixes without an
    issue and demonstration will not be accepted.
  • Please include: Fixes: #<issue number> in the commit message
  • please include tests. one line code fixes without tests will not be accepted.
• A new feature implementation
  • please include the issue number, and create an issue if none exists, which must
    include a complete example of how the feature would look.
  • Please include: Fixes: #<issue number> in the commit message
  • please include tests.

Have a nice day!

[zzzeek]

it's too bad because the colon isn't any problem for the revision API, it's just used by the command interface. colon could be supported if the command interface had a function that parsed for quoted revision names. but then we'd probably want quotes added to the illegal chars also

[PatilHrushikesh]

Can you explain

it's just used by the command interface.

Yeah, and it's causing issue mentioned in ticket, is there better way to fix it?

quoted revision

How does quoted revision comes into the picture?

[zzzeek]

the colon is only parsed inside of command.py, it's not used in revision.py

we could also say that we would support this syntax:

alembic upgrade --sql "some:rev":ae5c

however, there's no compelling reason for that ...

[CaselIT]

I think the solution proposed in this PR is likely easier to implement. What do you think mike?

[zzzeek]

it is. can you add a changelog and do we have illegal char tests set up? we should

[CaselIT]

@PatilHrushikesh

• could you add an additional case (or edit an existing one) here

  alembic/tests/test_script_production.py

  Lines 347 to 390 in 9cc68f5

  	def test_illegal_revision_chars(self):
  	assert_raises_message(
  	util.CommandError,
  	r"Character\(s\) '-' not allowed in "
  	"revision identifier 'no-dashes'",
  	command.revision,
  	self.cfg,
  	message="some message",
  	rev_id="no-dashes",
  	)
  	assert not os.path.exists(
  	os.path.join(self.env.dir, "versions", "no-dashes_some_message.py")
  	)
  	assert_raises_message(
  	util.CommandError,
  	r"Character\(s\) '@' not allowed in "
  	"revision identifier 'no@atsigns'",
  	command.revision,
  	self.cfg,
  	message="some message",
  	rev_id="no@atsigns",
  	)
  	assert_raises_message(
  	util.CommandError,
  	r"Character\(s\) '-, @' not allowed in revision "
  	"identifier 'no@atsigns-ordashes'",
  	command.revision,
  	self.cfg,
  	message="some message",
  	rev_id="no@atsigns-ordashes",
  	)
  	assert_raises_message(
  	util.CommandError,
  	r"Character\(s\) '\+' not allowed in revision "
  	r"identifier 'no\+plussignseither'",
  	command.revision,
  	self.cfg,
  	message="some message",
  	rev_id="no+plussignseither",
  	)

  to have a :?
• could you add a changelog entry here? examples are here https://github.com/sqlalchemy/alembic/tree/3db374f19760320507a96443e29d19835654b2ec/docs/build/unreleased the number is the issue id

Thanks!

[zzzeek]

is this PR stalled ? it needs the additional change as well as a unit test