Skip to content

[Big change] Server refactor (Part 1) #858

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.

Sign up for GitHub

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

Merged
merged 15 commits into from
Oct 19, 2022
Merged

[Big change] Server refactor (Part 1) #858

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
80949fc
Refactoring server.py into more intentional files
JackUrb Sep 23, 2019
802c612
Splitting all handlers into socket and base handlers
JackUrb Sep 23, 2019
fb3541f
reapply all changes since server refactor
da-h May 30, 2022
75c63e3
server: simplify & fix get_visdom_path/_to
da-h May 30, 2022
26a9ef1
server: move LAYOUT_FILE to defaults.py
da-h May 30, 2022
8bdb9c1
server: cleanup of imports
da-h May 30, 2022
277bcbf
setup: adapt console_script to new file structure
da-h Jul 29, 2022
d3ce2d3
server: remove zmq (as it has not been used & zmq.ioloop is deprecated)
da-h Jul 29, 2022
b3d838a
server: (minor) changed order of imports
da-h Jul 29, 2022
f597e9e
server: move MAX_SOCKET_WAIT to defaults.py
da-h Jul 29, 2022
2bbe4c7
server: added implementation comments about Sockets/Handlers
da-h Jul 29, 2022
77d5931
server: renamed ClientSocketWrapper -> SocketWrapper to unify naming …
da-h Jul 29, 2022
fd70fcd
github: adapted workflows to new filestructur in py/visdom/server
da-h Jul 29, 2022
c573028
github: (may be reverted after merge) ensures that cypress tests on m…
da-h Jul 29, 2022
f54852f
release version 0.2.2
da-h Oct 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions .github/workflows/process-changes.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,31 @@ jobs:
run: |
git checkout PR-HEAD -- ./cypress

- name: Check file existence
id: check_files
run: |
if test "py/visdom/server.py"; then
echo '::set-output name=file_exists::true'
else
echo '::set-output name=file_exists::false'
fi

- name: Cypress test:init
if: steps.check_files.outputs.file_exists == 'true'
uses: cypress-io/github-action@v2
with:
install: false
start: python3 py/visdom/server.py -port 8098 -env_path /tmp
wait-on: 'http://localhost:8098'
spec: cypress/integration/*.init.js
- name: Cypress test:init
uses: cypress-io/github-action@v2
if: steps.check_files.outputs.file_exists != 'true'
with:
install: false
start: python3 py/visdom/server -port 8098 -env_path /tmp
wait-on: 'http://localhost:8098'
spec: cypress/integration/*.init.js

- uses: actions/upload-artifact@v2
with:
Expand All @@ -96,7 +114,7 @@ jobs:
uses: cypress-io/github-action@v2
with:
install: false
start: python3 py/visdom/server.py -port 8098 -env_path /tmp
start: python3 py/visdom/server -port 8098 -env_path /tmp
wait-on: 'http://localhost:8098'
spec: cypress/integration/screenshots.js

Expand All @@ -118,7 +136,7 @@ jobs:
uses: cypress-io/github-action@v2
with:
install: false
start: python3 py/visdom/server.py -port 8098 -env_path /tmp
start: python3 py/visdom/server -port 8098 -env_path /tmp
wait-on: 'http://localhost:8098'
config: ignoreTestFiles=screenshots.*

Expand Down
2 changes: 1 addition & 1 deletion py/visdom/VERSION
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.2.1
0.2.2
2 changes: 2 additions & 0 deletions py/visdom/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
# This source code is licensed under the license found in the
Copy link
Contributor

Choose a reason for hiding this comment

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

As an aside, it's not the best that the entirety of visdom lives in the __init__.py file too. For a future refactor 😉

Copy link
Contributor Author

@da-h da-h Oct 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, that's true; the python-client code grew out of control. ^^

Considering some of the issues, there are quite some bugs to fix there as well, but tons of tiny things to keep track of at the same time. Any Idea how to improve the code efficiently?

Copy link
Contributor

Choose a reason for hiding this comment

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

For things like this, I'd argue that visdom has reached a scale that we need to hit on more DevX and codebase quality. Before jumping into the small changes, reorganizing would come first. Then I'd probably want to start using typing, though that can happen in parallel with small changes (every time you take a pass through to fix a bug, doing code quality improvements in the same pass). Typing and testing make sure that as you're making changes, the code is to some degree keeping track of things by itself so you don't have to.

# LICENSE file in the root directory of this source tree.

from visdom.utils.shared_utils import get_new_window_id
from visdom import server
import os.path
import requests
import traceback
Expand Down
Loading