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

Removed unused functionality to save and load queue and thus dependency to redis #1312

Merged
merged 4 commits into from
Jul 11, 2024

Conversation

marcus-oscarsson
Copy link
Member

@marcus-oscarsson marcus-oscarsson commented Jul 11, 2024

This removes the unused functionality to save and load queue and thus the dependency to Redis.

I also wanted to edit conda-environemnt.yml but that gives me the following ymllint errors:

12:24     warning  too few spaces before comment  (comments)
18:25     warning  too few spaces before comment  (comments)
24:24     warning  too few spaces before comment  (comments)
25:16     warning  too few spaces before comment  (comments)

It somehow seems wrong because the other comments pass ?, I tried to fix it by adding spaces but that did not seem to fix the issue. Does anyone know whats wrong ?

I'm all for tightening up the linting rules, its perhaps because I don't know the rules yet and the ymllint tool but as it is now it start to quite annoying to work with.

@marcus-oscarsson
Copy link
Member Author

marcus-oscarsson commented Jul 11, 2024

So I manage to work around the ymllint error, it turns out that ymllint expects two spaces before inline comments. However prettier re-formats and removes all extra spaces before inline comments. So there is a conflicting rule/setting. Can we change the ymllint rule to only one space ?

@marcus-oscarsson marcus-oscarsson marked this pull request as ready for review July 11, 2024 10:25
@fabcor-maxiv
Copy link
Contributor

fabcor-maxiv commented Jul 11, 2024

two spaces before inline comments

This is what Black does:

Black [...] enforces two spaces between code and a comment on the same line

-- https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#comments

I guess we can change if we prefer Javascript/Prettier's style rather than Python/Black's style.

@axelboc
Copy link
Collaborator

axelboc commented Jul 11, 2024

It'd be best to avoid running both formatting/linting tools on the same file type and/or for checking the same rule. If ymllint already takes care of both linting and formatting, then perhaps it should be prioritised over Prettier.

EDIT: if so, then adding !**/*.yml to .prettierignore should do the trick.

@marcus-oscarsson
Copy link
Member Author

Yes disabling the prettier for YAML would be the better option

@fabcor-maxiv
Copy link
Contributor

I am confused why the prettier vs. yamllint conflict did not show up...

Let's fix this: #1313

@fabcor-maxiv
Copy link
Contributor

@marcus-oscarsson

Thanks for looking into this.

I will help with fixing the readthedocs config. I will try to get to it soon-ish, but it might be only next week, not sure.

@marcus-oscarsson
Copy link
Member Author

:) @marcus-oscarsson forgot about the docs

@fabcor-maxiv
Copy link
Contributor

@marcus-oscarsson Yes, the readthedocs config change seems fine, in principle it should now be more or less the same as for mxcubecore.

@marcus-oscarsson
Copy link
Member Author

@fabcor-maxiv: I did what I thought was the right thing, feel free to followup with a cleanup if needed

@marcus-oscarsson marcus-oscarsson merged commit 77d0806 into develop Jul 11, 2024
13 checks passed
@marcus-oscarsson marcus-oscarsson deleted the mo-redis branch July 11, 2024 14:21
@fabcor-maxiv
Copy link
Contributor

There seems to be an issue with this change. It is a non-strictly-blocking one (as we expected), since the automated tests -- both pytest and Cypress -- seem to pass.

This is what it looks like for me when I start MXCuBE-Web with the demo mockups:

/home/fabcor/.local/share/mamba/envs/mxcubeweb/bin/mxcubeweb-server --static-folder=/home/fabcor/workspace/mxcube/mxcubeweb/ui/build --repository=/home/fabcor/workspace/mxcube/mxcubeweb/test/HardwareObjectsMockup.xml
2024-07-11 16:26:19,191 |MX3.HWR|INFO   | MXCuBE 3 initialized, it took 0.4 seconds
Traceback (most recent call last):
  File "/home/fabcor/.local/share/mamba/envs/mxcubeweb/lib/python3.10/site-packages/redis/connection.py", line 707, in connect
    sock = self.retry.call_with_retry(
  File "/home/fabcor/.local/share/mamba/envs/mxcubeweb/lib/python3.10/site-packages/redis/retry.py", line 46, in call_with_retry
    return do()
  File "/home/fabcor/.local/share/mamba/envs/mxcubeweb/lib/python3.10/site-packages/redis/connection.py", line 708, in <lambda>
    lambda: self._connect(), lambda error: self.disconnect(error)
  File "/home/fabcor/.local/share/mamba/envs/mxcubeweb/lib/python3.10/site-packages/redis/connection.py", line 1006, in _connect
    raise err
  File "/home/fabcor/.local/share/mamba/envs/mxcubeweb/lib/python3.10/site-packages/redis/connection.py", line 994, in _connect
    sock.connect(socket_address)
  File "/home/fabcor/.local/share/mamba/envs/mxcubeweb/lib/python3.10/site-packages/gevent/_socketcommon.py", line 607, in connect
    raise _SocketError(err, strerror(err))
ConnectionRefusedError: [Errno 111] Connection refused

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "src/gevent/greenlet.py", line 906, in gevent._gevent_cgreenlet.Greenlet.run
  File "/home/fabcor/.local/share/mamba/envs/mxcubeweb/lib/python3.10/site-packages/mxcubecore/HardwareObjects/DataPublisher.py", line 111, in _handle_messages
    pubsub.psubscribe("HWR_DP_NEW_DATA_POINT_*")
  File "/home/fabcor/.local/share/mamba/envs/mxcubeweb/lib/python3.10/site-packages/redis/client.py", line 1595, in psubscribe
    ret_val = self.execute_command("PSUBSCRIBE", *new_patterns.keys())
  File "/home/fabcor/.local/share/mamba/envs/mxcubeweb/lib/python3.10/site-packages/redis/client.py", line 1469, in execute_command
    self.connection = self.connection_pool.get_connection(
  File "/home/fabcor/.local/share/mamba/envs/mxcubeweb/lib/python3.10/site-packages/redis/connection.py", line 1461, in get_connection
    connection.connect()
  File "/home/fabcor/.local/share/mamba/envs/mxcubeweb/lib/python3.10/site-packages/redis/connection.py", line 713, in connect
    raise ConnectionError(self._error_message(e))
redis.exceptions.ConnectionError: Error 111 connecting to localhost:6379. Connection refused.
2024-07-11T14:26:19Z <Greenlet at 0x7f06eb4b79a0: <bound method DataPublisher._handle_messages of <DataPublisher.DataPublisher object at 0x7f070735f220>>> failed with ConnectionError

@marcus-oscarsson
Copy link
Member Author

marcus-oscarsson commented Jul 11, 2024

Yes this is entirely "normal" as DataPublisher depends on redis and testing and using that mockup will always require redis.

@marcus-oscarsson
Copy link
Member Author

If you wish to use the mockups without it you need to rename the data_publisher.xml to i.e data_publisher.xml.nu or something

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.

3 participants