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

Clarify how dependencies are defined #3702

Closed
PyDeps opened this issue Oct 26, 2022 · 4 comments · Fixed by #3879
Closed

Clarify how dependencies are defined #3702

PyDeps opened this issue Oct 26, 2022 · 4 comments · Fixed by #3879
Labels

Comments

@PyDeps
Copy link

PyDeps commented Oct 26, 2022

Hi, In molecule, inappropriate dependency versioning constraints can cause risks.

Below are the dependencies and version constraints that the project is using

alabaster==0.7.12
ansi2html==1.8.0
ansible-compat==2.2.0
ansible-core==2.13.4
ansible-pygments==0.1.1
arrow==1.2.3
attrs==22.1.0
babel==2.10.3
binaryornot==0.4.4
certifi==2022.6.15
cffi==1.15.1
chardet==5.0.0
charset-normalizer==2.1.1
click==8.1.3
click-help-colors==0.9.1
commonmark==0.9.1
cookiecutter==2.1.1
coverage==6.4.4
cryptography==37.0.4
distro==1.7.0
docutils==0.17.1
enrich==1.2.7
execnet==1.9.0
filelock==3.8.0
idna==3.3
imagesize==1.4.1
importlib-metadata==4.12.0
importlib-resources==5.9.0
iniconfig==1.1.1
jinja2==3.1.2
jinja2-time==0.2.0
jsonschema==4.16.0
markupsafe==2.1.1
more-itertools==8.14.0
packaging==21.3
pexpect==4.8.0
pkgutil-resolve-name==1.3.10
pluggy==1.0.0
ptyprocess==0.7.0
py==1.11.0
pycparser==2.21
pygments==2.13.0
pyparsing==3.0.9
pyrsistent==0.18.1
pytest==7.1.3
pytest-cov==3.0.0
pytest-forked==1.4.0
pytest-html==3.1.1
pytest-metadata==2.0.2
pytest-mock==3.8.2
pytest-plus==0.2
pytest-testinfra==6.8.0
pytest-xdist==2.5.0
python-dateutil==2.8.2
python-slugify==6.1.2
pytz==2022.2.1
pyyaml==6.0
requests==2.28.1
resolvelib==0.8.1
rich==12.5.1
simplejson==3.17.6
six==1.16.0
snowballstemmer==2.2.0
sphinx==5.1.1
sphinx-ansible-theme==0.9.1
sphinx-notfound-page==0.8.3
sphinx-rtd-theme==1.0.0
sphinxcontrib-applehelp==1.0.2
sphinxcontrib-devhelp==1.0.2
sphinxcontrib-htmlhelp==2.0.0
sphinxcontrib-jsmath==1.0.1
sphinxcontrib-qthelp==1.0.3
sphinxcontrib-serializinghtml==1.1.5
subprocess-tee==0.3.5
text-unidecode==1.3
tomli==2.0.1
typing-extensions==4.3.0
urllib3==1.26.12
zipp==3.8.1

The version constraint == will introduce the risk of dependency conflicts because the scope of dependencies is too strict.
The version constraint No Upper Bound and * will introduce the risk of the missing API Error because the latest version of the dependencies may remove some APIs.

After further analysis, in this project,
The version constraint of dependency ansible-compat can be changed to >=0.2.0,<=2.2.1.
The version constraint of dependency cookiecutter can be changed to >=0.5,<=2.1.1.
The version constraint of dependency enrich can be changed to >=1.1,<=1.2.7.
The version constraint of dependency jsonschema can be changed to >=2.0.0,<=4.6.0.
The version constraint of dependency pluggy can be changed to >=0.3.0,<=1.0.0.dev0.
The version constraint of dependency rich can be changed to >=0.2.0,<=12.6.0a2.

The above modification suggestions can reduce the dependency conflicts as much as possible,
and introduce the latest version as much as possible without calling Error in the projects.

The invocation of the current project includes all the following methods.

The calling methods from the ansible-compat
ansible_compat.runtime.Runtime
The calling methods from the cookiecutter
cookiecutter.main.cookiecutter
The calling methods from the enrich
enrich.logging.RichHandler
The calling methods from the jsonschema
jsonschema.validate
The calling methods from the pluggy
pluggy.PluginManager.load_setuptools_entrypoints
pluggy.PluginManager.get_plugins
pluggy.PluginManager
pluggy.PluginManager.get_name
The calling methods from the rich
rich.theme.Theme
rich.table.Table.add_column
rich.table.Table
rich.table.Table.add_row
rich.syntax.Syntax
The calling methods from the all methods
self.options.get
molecule.command.base.execute_cmdline_scenarios
self.Shell.super.__init__
self._load_file
command
self._config.state.reset
rich.syntax.Syntax
os.environ.get
App
sysexit
inspect.getfile
t.from_string.from_string
collections.defaultdict
self.__str__
merge_dicts
options.get
repr
b.items
Role.execute
self._verify
setattr
warnings.warn
os.removedirs
molecule.util.safe_load_file
data.items
self._get_defaults
pathlib.Path
subprocess.run
console_options.copy
molecule.command.base.get_configs
d.Path.mkdir
InvalidInterpolation
super
uuid.uuid4
self.options.get.get
self.config.get
self._config.write
molecule.util.dict2args
copy.deepcopy
mo.group.startswith
self._get_ssh_connection_options.append
self.__dict__.get
molecule.config.Config
dict
self._get_instance_config.get
rich.table.Table.add_column
os.path.expanduser.seek
copy.deepcopy.keys
self._interpolate
list.append
task_line.re.search.groups
molecule.scenarios.Scenarios.sequence
invoker.execute
os.getcwd.split
jinja2.Environment
self._get_config
self._config.provisioner.abs_path
self._config.provisioner.converge
_verify_configs
molecule.config.molecule_directory
sysexit_with_message
molecule.scenario.Scenario
x.startswith
logging.getLogger.setLevel
self.connection_options
value.str.lower
self._write_state_file
self._get_plugin_directory
self.__module__.split
self._reget_config
frozenset.union
self.AnsibleGalaxyBase.super.__init__
self._default_to_regular.items
molecule_prepender
safe_dump
yaml.safe_load
molecule.command.base.click_command_ex
self._resolve_template_dir
logging.getLogger.critical
str.items
molecule.scenarios.Scenarios._get_matrix
self.Ansible.super.__init__
self._combine
molecule.util.safe_load_file.items
t.from_string.render
self.api.verifiers.get
molecule.text.strip_ansi_escape
scenario_names.collections.Counter.items
self._config.driver.get_playbook
LOG_LEVEL_LUT.get
molecule.interpolation.Interpolator
self._vivify
pluggy.PluginManager.get_plugins
self._created
self._get_modules_directories
re.sub
self._config.driver.login_cmd_template.format
type.__call__.after_init
molecule.api.drivers
molecule.util.merge
self.filter_options
self._config.provisioner.syntax
self._config.state.change_state
filter
molecule.util.verbose_flag
line.startswith
open_file
molecule.util.render_template
molecule.version
UserListMap.append
_print_tabulate_data
execute_subcommand
self._config.provisioner.destroy
os.path.join
self._link_or_update_vars
molecule.text.underscore
self._validate
list
molecule.text.camelize
click.Choice
self._remove_vars
self._get_bundled_driver_playbook
get_configs
molecule.logger.set_log_level
collections.Counter
logging.getLogger.warning
self._default_to_regular.modules_dir
os.path.isdir
value.str.lower.strip
self._get_playbook
sorted
molecule_directory
self._default_data
cmd_args.get
functools.wraps
atexit.register
os.makedirs
text.splitlines
time.time
result.append
c.get
self._config.provisioner.verify
main.add_command
molecule.scenarios.Scenarios
molecule.provisioner.ansible.Ansible
enrich.logging.RichHandler
subprocess.CalledProcessError
self._command_args.api.verifiers.template_dir
a.lower.lower
molecule.platforms.Platforms
os.path.basename
molecule.util.abs_path.extend
molecule.dependency.ansible_galaxy.AnsibleGalaxy
molecule.state.State
molecule.util.abs_path.append
key.env.split
fcntl.lockf
super.execute
self.args.get
UserListMap
sys.exit
self._command_args.api.drivers.template_dir
molecule.util.print_as_yaml
enumerate
self._get_filter_plugin_directory
click.option
command_args.get
os.path.abspath
glob.glob
len
print
re.search
ansible_env.copy.items
self._get_config_template
map
string.replace.lower
molecule.api.verifiers
molecule.util.abs_path
pluggy.PluginManager.load_setuptools_entrypoints
molecule.util._filter_platforms
list.extend
to_bool
isinstance
json.load
type.__call__
mapping.get
shutil.rmtree
scenario._remove_scenario_state_directory
self._verify_inventory
self._config.driver.status
self._non_idempotent_tasks
self._config.dependency.execute
string.self.templater.substitute
bool
TypeError
should_do_markup
stream.read
warnings.filterwarnings
os.popen.read.split
os.popen.read
fnmatch.fnmatch
self._get_ansible_playbook.execute
os.getcwd
super.__init_subclass__
self.AnsibleGalaxy.super.__init__
molecule.scenarios.Scenarios.print_matrix
rich.table.Table
self._has_requirements_file
parallelize
self._process_templates
self.links.items
driver.required_collections.items
os.path.isabs
molecule.dependency.shell.Shell
os.umask
molecule.logger.get_section_loggers
self.Delegated.super.__init__
Scenario.execute
x.rstrip
os.mkdir
logging.getLogger.debug
ephemeral_directory
self._config.provisioner.check
self.add_cli_arg
platform.get
int
self._config.driver.required_collections.items
logging.getLogger.addHandler
self._config.config.get
self.abs_path
self.SafeDumper.super.increase_indent
env.items.copy
rich.syntax.Syntax.append
warnings.catch_warnings
init.add_command
self.host_vars.items
self._get_state_file
os.environ.split
molecule.command.base._get_subcommand
self._write_inventory
os.unlink
os.path.expanduser.write
self._config.command_args.get
mapping.get.startswith
molecule.util._parallelize_platforms
str.__lt__
self._get_driver_name
os.getenv
self._config.provisioner.prepare
self.UserListMap.super.__getitem__
frozenset
re.compile
_print_yaml_data
self._add_or_update_vars
sys.stdout.isatty
molecule.util.lookup_config_file
zip
find_vcs_root
molecule.util.open_file
molecule.model.schema_v3.validate
ctx.obj.get
join
pluggy.PluginManager.get_name
click.argument
ansible_env.copy.update
self._get_hostname
molecule.util.merge_dicts.get
self._get_ssh_connection_options
molecule.util.safe_load
importlib.metadata.version
molecule.util.BakedCommand
molecule.util.write_file
word.split
data.decode.decode
x.capitalize
os.walk
self._config.driver.login_options
self._is_idempotent
self.UserListMap.super.append
molecule.text.strip_ansi_escape.split
logging.getLogger
os.path.islink
molecule.util.validate_parallel_cmd_args
driver.reset
k.prefix.replace
str
molecule.console.console.export_html
Role
molecule.util.boolean
mo.group
self._config.runtime.require_collection
range
os.path.dirname
os.path.isfile
UserListMap.sort
molecule.config.command.execute
self._config.verifier.execute
m.group
execute_scenario
molecule.app.app.runtime.exec
time.sleep
SystemExit
copy.deepcopy.pop
self._get_ansible_playbook
rich.style.Style
dict.items
molecule.shell.main
self._get_instance_config
enrich.console.Console
self._config.provisioner.manage_inventory
strip
traceback.format_exc
packaging.version.Version
self._get_data
os.path.expanduser
self._config.provisioner.cleanup
molecule.console.should_do_markup
ansible_compat.runtime.Runtime
status_list.append
safe_load
self._converged
set_env_from_file
self._scenarios.pop
self._filter_for_scenario
jsonschema.validate
molecule.util.merge_dicts
self._get_playbook_directory
string.replace.replace
self._validate_template_dir
any
scenario.prune
self._default_to_regular
self._absolute_path_for
rich.table.Table.add_row
self.bake
self._config.driver.sanity_checks
wrapper
re.match
self._data.get
molecule.util.filter_verbose_permutation
scenario.config.command_args.get
molecule.util.sysexit_with_message
os.path.exists
molecule.command.base.execute_subcommand
self._config.provisioner.write_config
self._config.config.copy
molecule.interpolation.Interpolator.interpolate
res.append
Scenario
self.execute_with_retries
self._setup
self._filter_for_scenario.sort
glob_str.replace.replace
molecule.util.run_command
re.compile.match
self._config.state.converged.str.lower
os.path.expanduser.read
molecule.dependency.ansible_galaxy.collections.Collections
rich.theme.Theme
molecule.text.title
p
logging.getLogger.info
molecule.util.bool2args
pluggy.PluginManager
self._config.state.created.str.lower
print_debug
RuntimeError
open
molecule.util.safe_dump
self._config.provisioner.create
next
statuses.extend
self._get_login
cookiecutter.main.cookiecutter
ctx.exit
logging.getLogger.error
print_environment_vars
click.command
func
copy.copy
self._get_ansible_playbook.add_cli_arg
copy.copy.keys
self.Login.super.__init__
role_name.split
molecule.status.Status
shlex.split
invoker._has_requirements_file
string.split
getattr
os.path.expanduser.close
molecule.dependency.ansible_galaxy.roles.Roles
molecule.logger.configure
molecule.util.os_walk
self._normalize_playbook
molecule.util.sysexit
click.group
self.pattern.sub
molecule.console.console.print
subcommand_and_args.split
os.remove
os.symlink
self.templater
m.group.upper
self.command_args.get
self._invalid
yaml.dump
line.re.search.groups
self.Delegated.super._created
self._config.provisioner.side_effect
molecule.provisioner.ansible_playbook.AnsiblePlaybook
os.popen
self.name.__hash__
molecule.command.base.click_group_ex
mo.group.partition
self._config.driver.ansible_connection_options
datetime.date.today
self._config.project_directory.rsplit
scenario.config.runtime.prepare_environment
molecule.provisioner.ansible_playbooks.AnsiblePlaybooks

@developer
Could please help me check this issue?
May I pull a request to fix it?
Thank you very much.

@PyDeps PyDeps added the bug label Oct 26, 2022
@zhan9san
Copy link
Contributor

Hi @PyDeps

The requirements.txt(AKA constraint file) is for dependabot only.

The real dependency is defined in setup.cfg when users install this Python library. Do you still think it is too strict?

https://github.com/ansible-community/molecule/blob/8b0ef4f194230f4591ecc1643c93922ae5b400d9/setup.cfg#L60-L71

@davedittrich
Copy link
Contributor

Just wondering out loud here (so to speak). Having spent a lot of time looking through source code of several Ansible core and community repos lately, I've also noticed these differences between specified versions in multiple files in the same repo.

If something odd like this is being done on purpose ("for dependabot only", in this case), wouldn't documenting this prominently in the files themselves make the intention more clear and reduce situations where well-intentioned contributors invest a lot of time, possibly needlessly?

Could the requirements.txt and/or setup.py files be generated from a single source using a helper Makefile? That would be both more DRY and more clear about programmer intentions. Or a request to GitHub to support dependabot reading setup.py instead of only requirements.txt? An explanation in a comment to a PR or Issue is not really the ideal place for documenting source code.

@zhan9san
Copy link
Contributor

Sorry to make you confused.

If something odd like this is being done on purpose ("for dependabot only", in this case), wouldn't documenting this prominently in the files themselves

It is self-explanatory at the top of requirements.txt. I thought you may miss it.

The requirements.txt is generated by setup.cfg

https://github.com/ansible-community/molecule/blob/566f34ee7281e491aac9d9ea63f6e6f34e6583d5/requirements.txt#L1-L6

Could the requirements.txt and/or setup.py files be generated from a single source using a helper Makefile?

No setup.py in this repo.

Or a request to GitHub to support dependabot reading setup.py instead of only requirements.txt?

Supporting setup.py may not be a good way. Please refer to dependabot/dependabot-core#3290 (comment)

@davedittrich
Copy link
Contributor

No, I'm sorry. You did not confuse me. I only responded to the comment thread (without taking the additional time to look at the actual file) and thus I did miss (as you point out) the comment about auto-generation.

ssbarnea added a commit that referenced this issue Apr 20, 2023
ssbarnea added a commit that referenced this issue Apr 20, 2023
ssbarnea added a commit that referenced this issue Apr 20, 2023
ssbarnea added a commit that referenced this issue Apr 20, 2023
ssbarnea added a commit that referenced this issue Apr 20, 2023
@ssbarnea ssbarnea changed the title Project dependencies may have API risk issues Clarify how dependencies are defined Apr 20, 2023
ssbarnea added a commit that referenced this issue Apr 20, 2023
ssbarnea added a commit that referenced this issue Apr 20, 2023
ssbarnea added a commit that referenced this issue Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants