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

Add initial Windows support #29

Merged
merged 45 commits into from Oct 29, 2018

Conversation

@libre-man
Copy link
Contributor

commented Oct 17, 2018

Description

Add initial Windows support, this only works with the latest version of WinFsp (2018.2 B3) as only that release supports case sensitive directories. This still needs to check if the correct version of WinFsp is installed and display an error message if this is not the case. I still want to add a GUI, but that is for another PR.

The special files also need to be fixed (they are currently removed on all platforms).

UPDATE: This PR is now ready for merge, as it seems to work across platforms.

@libre-man libre-man changed the title Add initial Windows support [DO NOT MERGE] Add initial Windows support Oct 17, 2018

@libre-man

This comment has been minimized.

Copy link
Contributor Author

commented Oct 17, 2018

Another issue we have is that CodeGrade currently only supports python 3.7, but the FS still supports 3.5. However we can only test on 3.7, so we will need to fix that.

libre-man added some commits Oct 17, 2018

@codecov

This comment has been minimized.

Copy link

commented Oct 17, 2018

Codecov Report

Merging #29 into master will decrease coverage by 4.56%.
The diff coverage is 82.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #29      +/-   ##
==========================================
- Coverage   94.62%   90.05%   -4.57%     
==========================================
  Files           2        6       +4     
  Lines        1265     1539     +274     
==========================================
+ Hits         1197     1386     +189     
- Misses         68      153      +85
Impacted Files Coverage Δ
codegra_fs/__init__.py 100% <100%> (ø)
codegra_fs/constants.py 100% <100%> (ø)
codegra_fs/cgfs_types.py 100% <100%> (ø)
codegra_fs/utils.py 38.23% <38.23%> (ø)
codegra_fs/cgfs.py 90.45% <84.12%> (-3.79%) ⬇️
codegra_fs/cgapi.py 95.09% <85.18%> (-1.73%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ddc174...ffb4a29. Read the comment docs.

libre-man added some commits Oct 17, 2018


if sys.platform.startswith('win32'):
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.connect(('localhost', int(socetfile_content)))

This comment has been minimized.

Copy link
@olmokramer

olmokramer Oct 26, 2018

Contributor

s/socet/socket/


assigned_only_help = """Only show submissions that are assigned to you. This
only has effect if submissions are assigned and you are one of the
assignee's."""

This comment has been minimized.

Copy link
@olmokramer

olmokramer Oct 26, 2018

Contributor

s/assignee's/assignees/

@@ -0,0 +1,29 @@
fixed_mode_help = """Mount the original files as read only. It is still
possible to create new files, but it is not possible to alter existing

This comment has been minimized.

Copy link
@olmokramer

olmokramer Oct 26, 2018

Contributor

s/alter/alter or delete/


url_help = """The url to find the api. This defaults to
'https://codegra.de/api/v1/'. This is most likely
'https://{UNIVERSITY}.codegra.de/api/v1' where '{UNIVERSITY}' is your

This comment has been minimized.

Copy link
@olmokramer

olmokramer Oct 26, 2018

Contributor

It's a bit inconsistent with the preceding sentence that the trailing / is left off here.

This comment has been minimized.

Copy link
@libre-man

libre-man Oct 26, 2018

Author Contributor

Agree.

setup.py Outdated
long_description=open('README.md', 'r', encoding='utf-8').read(),
long_description_content_type="text/markdown",
packages=['codegra_fs'],
entry_points={
'console_scripts':
[
'cgfs = codegra_fs.cgfs:main',
'cgapi-consumer = codegra_fs.api_consumer:main'
'cgapi-consumer = codegra_fs.api_consumer:main',
'cgfs-gui = codegra_fs.gui:main',

This comment has been minimized.

Copy link
@olmokramer

olmokramer Oct 26, 2018

Contributor

It is convention to name the executable after the widget framework, e.g. cgfs-qt.

This comment has been minimized.

Copy link
@libre-man

libre-man Oct 26, 2018

Author Contributor

Sure, I prefer it this way (it is a bit clearer for users I think) but cgfs-qt is fine by me.

"""This file lets users edit rubrics.
The format is as follows:
file:
rubric file | ε
rubric file |

This comment has been minimized.

Copy link
@olmokramer

olmokramer Oct 26, 2018

Contributor

Why were these ε removed?

This comment has been minimized.

Copy link
@libre-man

libre-man Oct 26, 2018

Author Contributor

Because windows removed them... I will add them back.

self.dirty = False
self.stat: FullStat

This comment has been minimized.

Copy link
@olmokramer

olmokramer Oct 26, 2018

Contributor

Does python 3.5 support annotated variables?

This comment has been minimized.

Copy link
@libre-man

libre-man Oct 26, 2018

Author Contributor

Yes!

This comment has been minimized.

Copy link
@libre-man

libre-man Oct 26, 2018

Author Contributor

Or at least, I think so.

This comment has been minimized.

Copy link
@libre-man

libre-man Oct 27, 2018

Author Contributor

Nope it doesn't I will fix it everywhere :)

file = self.get_file(path, expect_type=SingleFile)
file = self.get_file_with_fh(path, fh)

if todo == 'fsync':

This comment has been minimized.

Copy link
@olmokramer

olmokramer Oct 26, 2018

Contributor

Maybe we should make these Enums

This comment has been minimized.

Copy link
@libre-man

libre-man Oct 26, 2018

Author Contributor

Yeah, probably better 👍

'You are running an outdated version of'
' CGFS, please consider upgrading.'
),
'You can do this at https://codegra.de/codegra_fs/latest/',

This comment has been minimized.

Copy link
@olmokramer

olmokramer Oct 26, 2018

Contributor

This URL gives a 404. You probably already know that, just mentioning it so we don't forget about it :)

This comment has been minimized.

Copy link
@libre-man

libre-man Oct 26, 2018

Author Contributor

Yeah, we need to add it when we actually release something :)

sys.exit(2)

check_version()

argparser = ArgumentParser(description='CodeGra.de file system')

This comment has been minimized.

Copy link
@olmokramer

olmokramer Oct 26, 2018

Contributor

Though not conventional, maybe we could add an epilog describing the environment variables. We don't have a man page where they would usually be listed, but they're super useful.

This comment has been minimized.

Copy link
@libre-man

libre-man Oct 26, 2018

Author Contributor

Yeah, that is probably a good idea.

This comment has been minimized.

Copy link
@libre-man

libre-man Oct 27, 2018

Author Contributor

But something for another PR I think.

This comment has been minimized.

Copy link
@olmokramer

olmokramer Oct 27, 2018

Contributor

Sure.

This comment has been minimized.

Copy link
@libre-man

libre-man Oct 28, 2018

Author Contributor

Maybe it is better to it here, you can decide. What should it contain?

This comment has been minimized.

Copy link
@olmokramer

olmokramer Oct 28, 2018

Contributor

I was thinking something very simple along the lines of

Environment variables
CGAPI_BASE_URL      Base URL of your CodeGrade instance
CGFS_PASSWORD       Your CodeGrade password

It could be a little more verbose but not much I'd say.

This comment has been minimized.

Copy link
@libre-man

libre-man Oct 28, 2018

Author Contributor

Ok seems fine, I will add it!

libre-man added some commits Oct 27, 2018

Fix logging
It is now possible to change log levels between runs.

@libre-man libre-man changed the title [DO NOT MERGE] Add initial Windows support Add initial Windows support Oct 28, 2018

@libre-man libre-man force-pushed the feature/windows-support branch from 50cfc5c to d61205f Oct 28, 2018

@libre-man libre-man force-pushed the feature/windows-support branch from d61205f to b2f90ac Oct 28, 2018

README.md Outdated
The basic used of the `cgfs` can be viewed by executing `cgfs --help`. The idea
behind `cgfs` is that you mount a CodeGra.de instance on you local computer, in
the mounted folder you can now browse, alter and delete files submitted by
yourself and people you have to grade.

### GUI usage
If you prefer to use a GUI you can use the `cgfs-qt` command. This is an simple

This comment has been minimized.

Copy link
@olmokramer

olmokramer Oct 28, 2018

Contributor

s/an simple/a simple/

README.md Outdated
The basic used of the `cgfs` can be viewed by executing `cgfs --help`. The idea
behind `cgfs` is that you mount a CodeGra.de instance on you local computer, in
the mounted folder you can now browse, alter and delete files submitted by
yourself and people you have to grade.

### GUI usage
If you prefer to use a GUI you can use the `cgfs-qt` command. This is an simple
GUI that is beta to use CodeGra.fs. To use the GUI fill in all required fields,

This comment has been minimized.

Copy link
@olmokramer

olmokramer Oct 28, 2018

Contributor

"that is beta to use CodeGra.fs" should be reworded.


all_submissions_help = """See all submissions not just the latest submissions
of students."""

cgfs_epilog="""possible environment variables

This comment has been minimized.

Copy link
@olmokramer

olmokramer Oct 28, 2018

Contributor

I would remove "possible" here or change it to "supported", add a colon to the end, and remove the empty line below this one.

codegra_fs/constants.py Show resolved Hide resolved
codegra_fs/constants.py Show resolved Hide resolved

libre-man added some commits Oct 28, 2018

@libre-man libre-man force-pushed the feature/windows-support branch from f965717 to d29c0f8 Oct 28, 2018

olmokramer and others added some commits Oct 28, 2018

Merge branch 'feature/windows-support' of git://github.com/CodeGra-de…
…/CodeGra.fs into feature/windows-support
@olmokramer
Copy link
Contributor

left a comment

Awesome!

@olmokramer olmokramer merged commit fa2fdb4 into master Oct 29, 2018

1 of 3 checks passed

codecov/patch 82.07% of diff hit (target 94.62%)
Details
codecov/project 90.05% (-4.57%) compared to 1ddc174
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@olmokramer olmokramer deleted the feature/windows-support branch Oct 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.